[Models] Re-architect Prompts
Categories
(Core :: Machine Learning: General, defect, P1)
Tracking
()
People
(Reporter: tetchart, Assigned: mzhang, NeedInfo)
References
Details
(Whiteboard: [aiasst])
Attachments
(4 files, 6 obsolete files)
We are running into an issue with system prompt bloat causing performance degradations. We need to re-architect how we use prompts such that we can continue to extend functionality without materially increasing bloat over time. Proposal is in the doc: https://docs.google.com/document/d/1_FUPdNDuHRsu_ndK_U7_TMEkXRnQ9DWKiM-rWcYV5ZY/edit?tab=t.0#heading=h.c5lpgpeodxpc
|
||
Updated•4 months ago
|
|
Reporter | |
Comment 1•4 months ago
|
||
|
|
Reporter | |
Updated•4 months ago
|
|
||
Updated•3 months ago
|
Previously contextual messages (real-time info, relevant memories) were
spliced immediately BEFORE the last user message. Moving them AFTER
keeps the user's literal question first and the supporting context as
background the model can refer to. Validated via diff_regression and
human_eval_auto eval suites for qwen and gpt-oss; minor positive impact
on context_aware and follow-up scoring without negative side-effects.
|
|
||
Updated•3 months ago
|
|
|
||
Updated•3 months ago
|
|
|
||
Updated•3 months ago
|
Restructure D298594 into Patch A (this) + Patch B (later). Per the
2026-05-20 audit memo at browser/components/aiwindow/docs/chat-vs-non-chat-audit.md,
this patch implements the R1 recommendation: keep chat distinct,
unify the shared engine-call surface as a CallContext.
Changes:
-
New PromptLoader.sys.mjs exports loadCallContext(feature) and
loadPrompt(feature). loadCallContext returns
{model, parameters, serviceType, purpose} from Remote Settings.
loadPrompt returns the prompt string for a feature, honoring the
browser.smartwindow.customPrompts pref override. -
openAIEngine.build({model, serviceType, purpose, flowId, feature})
is now the only signature. The legacy build(feature, flowId) form
and engine.loadConfig / getConfig / loadPrompt methods are removed.
build no longer reads from Remote Settings; a regression-guard test
(test_engine_build_object_form.js) spies getRemoteClient and asserts
zero calls. -
Every LLM caller migrates to the new pattern: loadCallContext +
loadPrompt + object-form build. Sites updated: Chat.fetchWithHistory,
ChatConversation, ai-window.mjs chat entrypoint, TitleGeneration,
ConversationSuggestions (starters + followups), Memories helpers,
MemoriesManager. IntentClassifier uses a local ONNX engine and is
out of scope. -
ChatConversation.getRealTimeInfo and getMemoriesContext shed their
unused engineInstance parameter. -
Test stubs (head.js, test_Chat.js, test_Memories.js,
browser_utils_fxAccount.js, browser_conversation_stream.js) migrated
to the object form. test_PromptLoader_loadCallContext.js exercises
the new module against fixture records. -
FEATURE_MAJOR_VERSIONS gains entries for REAL_TIME_CONTEXT_{DATE,TAB,
MENTIONS} and ENABLE/DISABLE_TABLE_INSTRUCTIONS — these fragments
were previously loaded implicitly via additional_components but now
go through the standalone loadPrompt path.
The audit memo's Decision line is left as TBD pending Tyler's review.
D298594 is left as-is; this is a fresh revision under the same bug.
Verified end-to-end via ml_driver chat_eval smoke (1 case, Qwen,
no Patch A regression strings).
Per Tyler's review feedback on D301647 (R2 follow-up): replace
loadCallContext + openAIEngine.build({...}) + ad-hoc message arrays
with a single LLM object that encapsulates feature, model, serviceType,
purpose, parameters, flowId, engine, and accumulating messages.
PromptLoader.buildLLM(feature, opts) is now the single factory used by
chat (ai-window.mjs), title generation, memories generation/classification,
conversation suggestions, and follow-up prompts. loadPrompt(feature)
stays — still needed for template-only loading where the prompt text is
embedded in another LLM call.
This unification sets up future agents/chat/memories/title-gen to share
prompt compaction, debug serialization, and tool-loop infrastructure
without per-caller plumbing, as discussed during D301647 review.
Also fixes test-fixture drift in ui/test/browser/head.js MOCK_RS_RECORDS
(chat major version bump, generic model fallback, gemini model name) and
one stale getRealTimeInfo arg-index assertion in browser_smartwindow_
retry_context.js — both surfaced when buildLLM tightened version + purpose
resolution.
|
||
Comment 8•3 months ago
|
||
Backed out for causing failures at browser_aichat_content_message_event.js.
Backout link: https://hg.mozilla.org/integration/autoland/rev/47942f71f010
Teach PromptLoader to assemble the chat system prompt from modular v2 records
(identity, model-details, style, skills, trust-and-safety, response-rules,
browser-context) when the new browser.smartwindow.prompts.v2.dump pref points
at a JSON dump of the ai-window-prompts Remote Settings collection. Falls back
fragment-by-fragment to legacy v1 records when v2 is absent, so empty-dump
behavior is byte-identical to the prior patch.
New PromptLoader exports: buildChatSystemPrompt, buildBrowserContextPrompt,
getSkillPrompt. A new get_skill tool is surfaced when the v2 dump pref is set
and filtered out by Chat.filterFeatureGatedTools when empty.
ChatConversation.getRealTimeInfo collapses to a thin wrapper around
buildBrowserContextPrompt. loadPrompt(CHAT) is rewired to try v2 first with
legacy fallback and shared {tableInstructions} + {skill_list} substitution.
|
||
Comment 10•3 months ago
|
||
|
||
Updated•3 months ago
|
|
||
Comment 11•3 months ago
|
||
| bugherder | ||
|
|
||
Updated•2 months ago
|
|
|
||
Updated•2 months ago
|
|
|
||
Comment 12•2 months ago
|
||
|
||
Comment 13•2 months ago
|
||
| bugherder | ||
|
|
||
Updated•2 months ago
|
|
|
Reporter | |
Comment 14•2 months ago
|
||
Copying most relevant detail from gdoc https://docs.google.com/document/d/1_FUPdNDuHRsu_ndK_U7_TMEkXRnQ9DWKiM-rWcYV5ZY/edit?tab=t.0#heading=h.c5lpgpeodxpc:
We will have 3 prompting systems in place:
Modular System Prompt
User Browser Context
Assistant Retrieved Skills
Modular System Prompt
The modular system prompt is modular only in development. The form of the system prompt at runtime will be all concatenated together as a single prompt with the role of “system”.
It is modular in design for 2 reasons
Minimize prompt size: Each module will have a specific purpose. We will minimize the instructions by staying focused on that purpose. Any net-new instructions will either need to justify a new module, or more likely, be added as a skill
Model tuned prompt swaps: Model tuning prompts will still happen, but they will be much more focused on individual prompt modules rather than the entire system prompt at once. They will be focused and only applied as needed, with most models getting the default “generic” prompt
The only exception to this is the model specific instruction module. This is where things like model name and knowledge cutoff will live. Default is empty for this module
The modules proposed are the following, and will be constructed in this order
Identity
What is Smart Window?
Model Details
Specific details for models. Default is nothing here. Things like model name and knowledge cutoff date
Style
What is the “personality” of Smart Window like? Tone, persona, etc.
Skills
Brief description of the purpose of skills and a variable placeholder for the list of skills pulled from remote settings (name and description for each)
Trust and Safety
T&S prompt guardrails
Response Rules
Specific rules around how SW should respond. Things like source citation, search suggestions, formatting, etc. should go here
Browser Context
Static context about the browser. Today that is time, date, and locale
Note that tabs and mentions don’t go here because they will be dynamically applied throughout the conversation
Once created, the system prompt will be static for the course of the conversation. This is to reduce prompt cache busting
User Browser Context
The user will have browser context that SW should be aware of that is dynamic in a conversation. Today that is active tab and @mentions. These will be injected into the conversation as a “user” role prior to the real “user” message in convo history.
What differentiates this from existing implementation is the following:
Browser context should only be injected once unless changed. If the active tab is the same from turn 1 to turn 2, then we don’t need to reinject and we don’t need to rewrite the browser history.
Prompts are minimal and “feel” like they come from the user rather than extensions of the system prompt
Adding it to the system prompt doesn’t capture the dynamic nature of the context
We tried adding it as an assistant response and as a mocked tool response, but neither performed as well as injecting it as a user message
This is common practice for coding tools like VSCode or Cursor to inject page context as “user” role
Prompt is something like
This is my active tab:
URL: {url}
Page title: {title}
Description: {description}
If we want to add rules around how it should be used, it should go in the browser context module in the system prompt
Assistant Retrieved Skills
Skills are a way to extend the SW knowledge base without bloating the system prompt. The design around a skill looks like this:
Name: <clear name of the skill, it will be used to index the skill>
Description: <clear description of the skill and why SW would need to call it>
Prompt: <Full prompt representing the knowledge the skill needs to contain>
An example of a skill is Kit. We want to be able to talk intelligently around Kit as a Mozilla brand, but it is a relatively rare occurrence and it bloats the system prompt to include language around Kit. We can create a Kit skill like this:
Name: kit,
Description: Contains details around Mozilla’s Kit, which is a cute Firefox mascot,
Prompt: <Long form prompt with details>
Skills will be pulled into the conversation as the assistant sees fit by the LLM calling the get_skill(name) function.
We also want to test automatic skill retrieval based on a semantic search with a very high precision threshold for automated triggering of the get_skill(name) tool call. Whether it triggers or not, the LLM can still call get_skill(name) as needed.
|
|
||
Updated•2 months ago
|
|
||
Comment 15•1 month ago
|
||
sounds like the third patch will close this out
|
|
||
Updated•1 month ago
|
|
|
||
Comment 16•1 month ago
|
||
|
||
Comment 17•1 month ago
|
||
| bugherder | ||
|
||
Comment 18•1 month ago
|
||
Combines the original D302389 work (decouple CallContext + prompt loading
from openAIEngine) with the follow-up layering refactor from Tyler's
2026-06-03/04 design feedback. Drops the LLM call-wrapper class entirely;
restructures ChatConversation/ChatMessage into a layered inheritance with
a generic Conversation/Message base in models/ and chat-specific
subclasses in ui/modules/.
Layering
models/Message (new — generic base)
^
ui/modules/ChatMessage (chat-only fields)
models/Conversation (new — generic base; owns
engine, parameters, run)
^
ui/modules/ChatConversation (chat orchestration + UI;
composes EventEmitter)
The base does not extend EventEmitter and has no this.emit calls — chat
overrides handleChunk / receiveResponse / retryMessage / set messages /
getMessagesInChatCompletionsFormat with chat side effects (no no-op hooks).
Class moves / renames
- LLM (models/LLM.sys.mjs) — deleted
- MessageAccumulator (models/MessageAccumulator.sys.mjs) — replaced by
Conversation (models/Conversation.sys.mjs) - openAIEngine pulled out of models/Utils.sys.mjs into its own file
models/openAIEngine.sys.mjs; Utils re-exports it for test back-compat
with a comment pointing new callers at the new module - Remote Settings access (RS_AI_WINDOW_COLLECTION, getRemoteClient,
modelPrefObserver, _remoteClient cache) lives in models/Utils.sys.mjs;
openAIEngine is now strictly the LiteLLM-endpoint transport - MESSAGE_ROLE canonical definition lives in models/Conversation.sys.mjs;
ui/modules/ChatEnums.sys.mjs re-exports it (was previously duplicated) - PromptLoader.loadCallContext + buildLLM → buildConversation +
buildEngineForFeature helper returning {engine, parameters} for chat
(which keeps a persistent ChatConversation across turns and refreshes
engine per-turn). buildEngineForFeature resolves baseURL + apiKey via
openAIEngine.resolveEndpointConfig(modelChoiceId). - toWireFormat → getMessagesInChatCompletionsFormat on the base; chat
override applies URL token substitution and splices userContext
messages just before the last user message. The pre-refactor
getMessagesInOpenAiFormat name is gone — all callers renamed.
What moves to the base
Message: id, createdDate, ordinal, role, content, turnIndex,
parentMessageId, modelId, params, usage, toolCallId, toolName
Conversation: id, createdDate, updatedDate, feature, engine, parameters,
#messages, _minNextOrdinal, currentTurnIndex, addMessage(role, content,
turnIndex, opts), add*Message variants, setSystemMessage (idempotent),
retryMessage (generic truncate), compactChatCompletions,
getMessagesInChatCompletionsFormat, handleChunk (returns Boolean for
whether anything was extracted), receiveResponse (drain + flush, no
chat post-stream phases), run / runWithGenerator, systemPromptVersion
getter
What stays on ChatConversation
Chat-only fields: title, description, pageUrl, pageMeta, status,
securityProperties, urlToToken / tokenToUrl / #baseTokenCounts /
seenUrls, activeBranchTipMessageId, transientStarterUrl/Starters,
memoriesToggled
UI methods: renderState, addUIToolToCurrentMessage, updateToolUI; the
composed #emitter and on/off/emit forwarders
Chat orchestration:
- loadSystemPrompt(opts) — idempotent upsert. Calls loadPrompt directly
and writes body + RS-record version onto the system message content.
Called at init AND on model change. - injectRealTimeContext(message, opts) — leaf op. Owns fetch + render +
in-place mutation of message.content.userContext.realTimeContext.
Replaces static getRealTimeInfo. - injectMemoriesContext(message, prompt) — same shape for memories.
Replaces getMemoriesContext. - generatePrompt — tight sequencer: loadSystemPrompt → addUserMessage →
emit → injectRealTimeContext → injectMemoriesContext →
securityProperties.commit.
Chat overrides of base methods (call super + chat extras)
- _createMessage — factory hook; returns ChatMessage with convId set
- addAssistantMessage / addToolCallMessage — auto-populate modelId from
this.engine?.model when opts doesn't already provide it - retryMessage — captures ephemeral system messages first, then super.
Refreshes #updateActiveBranchTipMessageId() since base splices in
place (bypasses the setter) - handleChunk — calls consumeStreamChunk with this.tokenToUrl directly
(does not super through the base), then applies plainText/tokens and
emits message-update + ChatStore.updateConversation - receiveResponse — super.receiveResponse(stream, currentMessage); then
memory-id resolve, URL token strip, ChatStore.updateConversation,
message-complete emit - set messages — super + #updateActiveBranchTipMessageId
- getMessagesInChatCompletionsFormat — accepts {applyUrlTokens=true};
filters out empty-body assistant placeholders and legacy ephemeral
SYSTEM-role realtime/memories messages, splices userContext as USER
messages just before the last user message, resolves inline @mention
URLs, applies URL→token substitution via replaceUrlsWithTokens
Versioning + LLMaJ telemetry
RS-record version is NOT a Conversation field — D304293 (Bug 2044484)
landed the convention of storing version on the system message's
content (message.content.version). loadSystemPrompt writes it there.
Base Conversation exposes systemPromptVersion reading back from the
system message. TelemetryUtils.runLLMaJTelemetry loses its llm param
and reads conversation.engine?.model + conversation.systemPromptVersion
directly.
Call-site updates
- Chat.fetchWithHistory({conversation, browsingContext, mode, signal}) —
drops the llm param. Uses conversation.compactChatCompletions() +
conversation.runWithGenerator(opts). Uses conversation.engine?.model
for telemetry tool calls. - ai-window.mjs per-turn: const {engine, parameters} = await
buildEngineForFeature(MODEL_FEATURES.CHAT, opts); assigns onto the
persistent ChatConversation. Model-switch path calls
conversation.loadSystemPrompt({modelChoiceIdOverride}). - TitleGeneration / ConversationSuggestions / Memories / MemoriesManager
— use buildConversation + conversation.setSystemMessage /
addUserMessage / run(opts). Memory pipeline functions clearMessages()
between steps because they reuse one Conversation across
generation/dedup/filter. - MemoriesManager.ensureLLMForGeneration / ensureLLMForUsage →
ensureConversationForGeneration / ensureConversationForUsage.
Bug fixes uncovered during refactor
- Chat.sys.mjs addToolCallMessage call sites were passing 3 args
(content, currentTurn, toolRoleOpts) where the chat signature only
takes (content, toolOpts). toolRoleOpts was silently dropped and
currentTurn was being spread into the Message constructor as opts.
Fixed at all 5 sites; modelId is now auto-populated. - ChatConversation constructor now seeds
#updateActiveBranchTipMessageId() after super() so DB-restored
conversations have it set before the first turn (base constructor
assigned messages directly into the private array, bypassing the
chat setter).
Tests
- test_LLM.js / test_MessageAccumulator.js — deleted
- test_PromptLoader_buildLLM.js → test_PromptLoader_buildConversation.js
- test_Message.js (new), test_Conversation.js (new) — cover base classes
- test_ChatConversation.js — removed direct tests for static
getRealTimeInfo / instance getMemoriesContext; added equivalents for
injectRealTimeContext / injectMemoriesContext. - test_ChatSwitchModel.js — updateSystemPromptForModel → loadSystemPrompt
- test_Chat.js / browser_conversation_stream.js — replaced LLM
construction with a setupConversationForChat helper that assigns
conversation.engine - test_MemoriesManager.js — ensureLLMForUsage stubs renamed
- test_TelemetryUtils.js — fake conversation exposes
getMessagesInChatCompletionsFormat - browser_smartwindow_prompts.js, browser_smartwindow_retry_context.js —
stub names + arg-index assertions updated for the new instance methods - Tests that previously stubbed openAIEngine.getRemoteClient switch to a
_setRemoteClientForTesting / _clearRemoteClientForTesting test seam in
Utils.sys.mjs (openAIEngine no longer owns getRemoteClient). - Browser-test fixture ui/test/browser/head.js — uses the test seam for
the RS client and _setLoadPromptForTesting for the chat system prompt.
Test status
xpcshell: 51/51 pass.
mochitest-browser: 2354/2357 pass; the 3 failures are the
browser_smartwindow_sanitize.js suite-order flake (Bug 2006444 —
passes solo, fails in the full suite). No regressions.
End-to-end verification through ml_driver: ran user_journey happy path
on gpt-oss-120b. 14 scenarios, 45/45 turns successful, 0 inference errors,
mean LLM-judge score 3.29.
Original Revision: https://phabricator.services.mozilla.com/D302389
|
|
||
Updated•1 month ago
|
|
|
||
Comment 19•1 month ago
|
||
Combines the original D302389 work (decouple CallContext + prompt loading
from openAIEngine) with the follow-up layering refactor from Tyler's
2026-06-03/04 design feedback. Drops the LLM call-wrapper class entirely;
restructures ChatConversation/ChatMessage into a layered inheritance with
a generic Conversation/Message base in models/ and chat-specific
subclasses in ui/modules/.
Layering
models/Message (new — generic base)
^
ui/modules/ChatMessage (chat-only fields)
models/Conversation (new — generic base; owns
engine, parameters, run)
^
ui/modules/ChatConversation (chat orchestration + UI;
composes EventEmitter)
The base does not extend EventEmitter and has no this.emit calls — chat
overrides handleChunk / receiveResponse / retryMessage / set messages /
getMessagesInChatCompletionsFormat with chat side effects (no no-op hooks).
Class moves / renames
- LLM (models/LLM.sys.mjs) — deleted
- MessageAccumulator (models/MessageAccumulator.sys.mjs) — replaced by
Conversation (models/Conversation.sys.mjs) - openAIEngine pulled out of models/Utils.sys.mjs into its own file
models/openAIEngine.sys.mjs; Utils re-exports it for test back-compat
with a comment pointing new callers at the new module - Remote Settings access (RS_AI_WINDOW_COLLECTION, getRemoteClient,
modelPrefObserver, _remoteClient cache) lives in models/Utils.sys.mjs;
openAIEngine is now strictly the LiteLLM-endpoint transport - MESSAGE_ROLE canonical definition lives in models/Conversation.sys.mjs;
ui/modules/ChatEnums.sys.mjs re-exports it (was previously duplicated) - PromptLoader.loadCallContext + buildLLM → buildConversation +
buildEngineForFeature helper returning {engine, parameters} for chat
(which keeps a persistent ChatConversation across turns and refreshes
engine per-turn). buildEngineForFeature resolves baseURL + apiKey via
openAIEngine.resolveEndpointConfig(modelChoiceId). - toWireFormat → getMessagesInChatCompletionsFormat on the base; chat
override applies URL token substitution and splices userContext
messages just before the last user message. The pre-refactor
getMessagesInOpenAiFormat name is gone — all callers renamed.
What moves to the base
Message: id, createdDate, ordinal, role, content, turnIndex,
parentMessageId, modelId, params, usage, toolCallId, toolName
Conversation: id, createdDate, updatedDate, feature, engine, parameters,
#messages, _minNextOrdinal, currentTurnIndex, addMessage(role, content,
turnIndex, opts), add*Message variants, setSystemMessage (idempotent),
retryMessage (generic truncate), compactChatCompletions,
getMessagesInChatCompletionsFormat, handleChunk (returns Boolean for
whether anything was extracted), receiveResponse (drain + flush, no
chat post-stream phases), run / runWithGenerator, systemPromptVersion
getter
What stays on ChatConversation
Chat-only fields: title, description, pageUrl, pageMeta, status,
securityProperties, urlToToken / tokenToUrl / #baseTokenCounts /
seenUrls, activeBranchTipMessageId, transientStarterUrl/Starters,
memoriesToggled
UI methods: renderState, addUIToolToCurrentMessage, updateToolUI; the
composed #emitter and on/off/emit forwarders
Chat orchestration:
- loadSystemPrompt(opts) — idempotent upsert. Calls loadPrompt directly
and writes body + RS-record version onto the system message content.
Called at init AND on model change. - injectRealTimeContext(message, opts) — leaf op. Owns fetch + render +
in-place mutation of message.content.userContext.realTimeContext.
Replaces static getRealTimeInfo. - injectMemoriesContext(message, prompt) — same shape for memories.
Replaces getMemoriesContext. - generatePrompt — tight sequencer: loadSystemPrompt → addUserMessage →
emit → injectRealTimeContext → injectMemoriesContext →
securityProperties.commit.
Chat overrides of base methods (call super + chat extras)
- _createMessage — factory hook; returns ChatMessage with convId set
- addAssistantMessage / addToolCallMessage — auto-populate modelId from
this.engine?.model when opts doesn't already provide it - retryMessage — captures ephemeral system messages first, then super.
Refreshes #updateActiveBranchTipMessageId() since base splices in
place (bypasses the setter) - handleChunk — calls consumeStreamChunk with this.tokenToUrl directly
(does not super through the base), then applies plainText/tokens and
emits message-update + ChatStore.updateConversation - receiveResponse — super.receiveResponse(stream, currentMessage); then
memory-id resolve, URL token strip, ChatStore.updateConversation,
message-complete emit - set messages — super + #updateActiveBranchTipMessageId
- getMessagesInChatCompletionsFormat — accepts {applyUrlTokens=true};
filters out empty-body assistant placeholders and legacy ephemeral
SYSTEM-role realtime/memories messages, splices userContext as USER
messages just before the last user message, resolves inline @mention
URLs, applies URL→token substitution via replaceUrlsWithTokens
Versioning + LLMaJ telemetry
RS-record version is NOT a Conversation field — D304293 (Bug 2044484)
landed the convention of storing version on the system message's
content (message.content.version). loadSystemPrompt writes it there.
Base Conversation exposes systemPromptVersion reading back from the
system message. TelemetryUtils.runLLMaJTelemetry loses its llm param
and reads conversation.engine?.model + conversation.systemPromptVersion
directly.
Call-site updates
- Chat.fetchWithHistory({conversation, browsingContext, mode, signal}) —
drops the llm param. Uses conversation.compactChatCompletions() +
conversation.runWithGenerator(opts). Uses conversation.engine?.model
for telemetry tool calls. - ai-window.mjs per-turn: const {engine, parameters} = await
buildEngineForFeature(MODEL_FEATURES.CHAT, opts); assigns onto the
persistent ChatConversation. Model-switch path calls
conversation.loadSystemPrompt({modelChoiceIdOverride}). - TitleGeneration / ConversationSuggestions / Memories / MemoriesManager
— use buildConversation + conversation.setSystemMessage /
addUserMessage / run(opts). Memory pipeline functions clearMessages()
between steps because they reuse one Conversation across
generation/dedup/filter. - MemoriesManager.ensureLLMForGeneration / ensureLLMForUsage →
ensureConversationForGeneration / ensureConversationForUsage.
Bug fixes uncovered during refactor
- Chat.sys.mjs addToolCallMessage call sites were passing 3 args
(content, currentTurn, toolRoleOpts) where the chat signature only
takes (content, toolOpts). toolRoleOpts was silently dropped and
currentTurn was being spread into the Message constructor as opts.
Fixed at all 5 sites; modelId is now auto-populated. - ChatConversation constructor now seeds
#updateActiveBranchTipMessageId() after super() so DB-restored
conversations have it set before the first turn (base constructor
assigned messages directly into the private array, bypassing the
chat setter).
Tests
- test_LLM.js / test_MessageAccumulator.js — deleted
- test_PromptLoader_buildLLM.js → test_PromptLoader_buildConversation.js
- test_Message.js (new), test_Conversation.js (new) — cover base classes
- test_ChatConversation.js — removed direct tests for static
getRealTimeInfo / instance getMemoriesContext; added equivalents for
injectRealTimeContext / injectMemoriesContext. - test_ChatSwitchModel.js — updateSystemPromptForModel → loadSystemPrompt
- test_Chat.js / browser_conversation_stream.js — replaced LLM
construction with a setupConversationForChat helper that assigns
conversation.engine - test_MemoriesManager.js — ensureLLMForUsage stubs renamed
- test_TelemetryUtils.js — fake conversation exposes
getMessagesInChatCompletionsFormat - browser_smartwindow_prompts.js, browser_smartwindow_retry_context.js —
stub names + arg-index assertions updated for the new instance methods - Tests that previously stubbed openAIEngine.getRemoteClient switch to a
_setRemoteClientForTesting / _clearRemoteClientForTesting test seam in
Utils.sys.mjs (openAIEngine no longer owns getRemoteClient). - Browser-test fixture ui/test/browser/head.js — uses the test seam for
the RS client and _setLoadPromptForTesting for the chat system prompt.
Test status
xpcshell: 51/51 pass.
mochitest-browser: 2354/2357 pass; the 3 failures are the
browser_smartwindow_sanitize.js suite-order flake (Bug 2006444 —
passes solo, fails in the full suite). No regressions.
End-to-end verification through ml_driver: ran user_journey happy path
on gpt-oss-120b. 14 scenarios, 45/45 turns successful, 0 inference errors,
mean LLM-judge score 3.29.
Original Revision: https://phabricator.services.mozilla.com/D302389
|
|
||
Updated•1 month ago
|
|
|
||
Comment 20•1 month ago
|
||
Combines the original D302389 work (decouple CallContext + prompt loading
from openAIEngine) with the follow-up layering refactor from Tyler's
2026-06-03/04 design feedback. Drops the LLM call-wrapper class entirely;
restructures ChatConversation/ChatMessage into a layered inheritance with
a generic Conversation/Message base in models/ and chat-specific
subclasses in ui/modules/.
Layering
models/Message (new — generic base)
^
ui/modules/ChatMessage (chat-only fields)
models/Conversation (new — generic base; owns
engine, parameters, run)
^
ui/modules/ChatConversation (chat orchestration + UI;
composes EventEmitter)
The base does not extend EventEmitter and has no this.emit calls — chat
overrides handleChunk / receiveResponse / retryMessage / set messages /
getMessagesInChatCompletionsFormat with chat side effects (no no-op hooks).
Class moves / renames
- LLM (models/LLM.sys.mjs) — deleted
- MessageAccumulator (models/MessageAccumulator.sys.mjs) — replaced by
Conversation (models/Conversation.sys.mjs) - openAIEngine pulled out of models/Utils.sys.mjs into its own file
models/openAIEngine.sys.mjs; Utils re-exports it for test back-compat
with a comment pointing new callers at the new module - Remote Settings access (RS_AI_WINDOW_COLLECTION, getRemoteClient,
modelPrefObserver, _remoteClient cache) lives in models/Utils.sys.mjs;
openAIEngine is now strictly the LiteLLM-endpoint transport - MESSAGE_ROLE canonical definition lives in models/Conversation.sys.mjs;
ui/modules/ChatEnums.sys.mjs re-exports it (was previously duplicated) - PromptLoader.loadCallContext + buildLLM → buildConversation +
buildEngineForFeature helper returning {engine, parameters} for chat
(which keeps a persistent ChatConversation across turns and refreshes
engine per-turn). buildEngineForFeature resolves baseURL + apiKey via
openAIEngine.resolveEndpointConfig(modelChoiceId). - toWireFormat → getMessagesInChatCompletionsFormat on the base; chat
override applies URL token substitution and splices userContext
messages just before the last user message. The pre-refactor
getMessagesInOpenAiFormat name is gone — all callers renamed.
What moves to the base
Message: id, createdDate, ordinal, role, content, turnIndex,
parentMessageId, modelId, params, usage, toolCallId, toolName
Conversation: id, createdDate, updatedDate, feature, engine, parameters,
#messages, _minNextOrdinal, currentTurnIndex, addMessage(role, content,
turnIndex, opts), add*Message variants, setSystemMessage (idempotent),
retryMessage (generic truncate), compactChatCompletions,
getMessagesInChatCompletionsFormat, handleChunk (returns Boolean for
whether anything was extracted), receiveResponse (drain + flush, no
chat post-stream phases), run / runWithGenerator, systemPromptVersion
getter
What stays on ChatConversation
Chat-only fields: title, description, pageUrl, pageMeta, status,
securityProperties, urlToToken / tokenToUrl / #baseTokenCounts /
seenUrls, activeBranchTipMessageId, transientStarterUrl/Starters,
memoriesToggled
UI methods: renderState, addUIToolToCurrentMessage, updateToolUI; the
composed #emitter and on/off/emit forwarders
Chat orchestration:
- loadSystemPrompt(opts) — idempotent upsert. Calls loadPrompt directly
and writes body + RS-record version onto the system message content.
Called at init AND on model change. - injectRealTimeContext(message, opts) — leaf op. Owns fetch + render +
in-place mutation of message.content.userContext.realTimeContext.
Replaces static getRealTimeInfo. - injectMemoriesContext(message, prompt) — same shape for memories.
Replaces getMemoriesContext. - generatePrompt — tight sequencer: loadSystemPrompt → addUserMessage →
emit → injectRealTimeContext → injectMemoriesContext →
securityProperties.commit.
Chat overrides of base methods (call super + chat extras)
- _createMessage — factory hook; returns ChatMessage with convId set
- addAssistantMessage / addToolCallMessage — auto-populate modelId from
this.engine?.model when opts doesn't already provide it - retryMessage — captures ephemeral system messages first, then super.
Refreshes #updateActiveBranchTipMessageId() since base splices in
place (bypasses the setter) - handleChunk — calls consumeStreamChunk with this.tokenToUrl directly
(does not super through the base), then applies plainText/tokens and
emits message-update + ChatStore.updateConversation - receiveResponse — super.receiveResponse(stream, currentMessage); then
memory-id resolve, URL token strip, ChatStore.updateConversation,
message-complete emit - set messages — super + #updateActiveBranchTipMessageId
- getMessagesInChatCompletionsFormat — accepts {applyUrlTokens=true};
filters out empty-body assistant placeholders and legacy ephemeral
SYSTEM-role realtime/memories messages, splices userContext as USER
messages just before the last user message, resolves inline @mention
URLs, applies URL→token substitution via replaceUrlsWithTokens
Versioning + LLMaJ telemetry
RS-record version is NOT a Conversation field — D304293 (Bug 2044484)
landed the convention of storing version on the system message's
content (message.content.version). loadSystemPrompt writes it there.
Base Conversation exposes systemPromptVersion reading back from the
system message. TelemetryUtils.runLLMaJTelemetry loses its llm param
and reads conversation.engine?.model + conversation.systemPromptVersion
directly.
Call-site updates
- Chat.fetchWithHistory({conversation, browsingContext, mode, signal}) —
drops the llm param. Uses conversation.compactChatCompletions() +
conversation.runWithGenerator(opts). Uses conversation.engine?.model
for telemetry tool calls. - ai-window.mjs per-turn: const {engine, parameters} = await
buildEngineForFeature(MODEL_FEATURES.CHAT, opts); assigns onto the
persistent ChatConversation. Model-switch path calls
conversation.loadSystemPrompt({modelChoiceIdOverride}). - TitleGeneration / ConversationSuggestions / Memories / MemoriesManager
— use buildConversation + conversation.setSystemMessage /
addUserMessage / run(opts). Memory pipeline functions clearMessages()
between steps because they reuse one Conversation across
generation/dedup/filter. - MemoriesManager.ensureLLMForGeneration / ensureLLMForUsage →
ensureConversationForGeneration / ensureConversationForUsage.
Bug fixes uncovered during refactor
- Chat.sys.mjs addToolCallMessage call sites were passing 3 args
(content, currentTurn, toolRoleOpts) where the chat signature only
takes (content, toolOpts). toolRoleOpts was silently dropped and
currentTurn was being spread into the Message constructor as opts.
Fixed at all 5 sites; modelId is now auto-populated. - ChatConversation constructor now seeds
#updateActiveBranchTipMessageId() after super() so DB-restored
conversations have it set before the first turn (base constructor
assigned messages directly into the private array, bypassing the
chat setter).
Tests
- test_LLM.js / test_MessageAccumulator.js — deleted
- test_PromptLoader_buildLLM.js → test_PromptLoader_buildConversation.js
- test_Message.js (new), test_Conversation.js (new) — cover base classes
- test_ChatConversation.js — removed direct tests for static
getRealTimeInfo / instance getMemoriesContext; added equivalents for
injectRealTimeContext / injectMemoriesContext. - test_ChatSwitchModel.js — updateSystemPromptForModel → loadSystemPrompt
- test_Chat.js / browser_conversation_stream.js — replaced LLM
construction with a setupConversationForChat helper that assigns
conversation.engine - test_MemoriesManager.js — ensureLLMForUsage stubs renamed
- test_TelemetryUtils.js — fake conversation exposes
getMessagesInChatCompletionsFormat - browser_smartwindow_prompts.js, browser_smartwindow_retry_context.js —
stub names + arg-index assertions updated for the new instance methods - Tests that previously stubbed openAIEngine.getRemoteClient switch to a
_setRemoteClientForTesting / _clearRemoteClientForTesting test seam in
Utils.sys.mjs (openAIEngine no longer owns getRemoteClient). - Browser-test fixture ui/test/browser/head.js — uses the test seam for
the RS client and _setLoadPromptForTesting for the chat system prompt.
Test status
xpcshell: 51/51 pass.
mochitest-browser: 2354/2357 pass; the 3 failures are the
browser_smartwindow_sanitize.js suite-order flake (Bug 2006444 —
passes solo, fails in the full suite). No regressions.
End-to-end verification through ml_driver: ran user_journey happy path
on gpt-oss-120b. 14 scenarios, 45/45 turns successful, 0 inference errors,
mean LLM-judge score 3.29.
Original Revision: https://phabricator.services.mozilla.com/D302389
|
|
||
Updated•1 month ago
|
|
|
||
Comment 21•1 month ago
|
||
Combines the original D302389 work (decouple CallContext + prompt loading
from openAIEngine) with the follow-up layering refactor from Tyler's
2026-06-03/04 design feedback. Drops the LLM call-wrapper class entirely;
restructures ChatConversation/ChatMessage into a layered inheritance with
a generic Conversation/Message base in models/ and chat-specific
subclasses in ui/modules/.
Layering
models/Message (new — generic base)
^
ui/modules/ChatMessage (chat-only fields)
models/Conversation (new — generic base; owns
engine, parameters, run)
^
ui/modules/ChatConversation (chat orchestration + UI;
composes EventEmitter)
The base does not extend EventEmitter and has no this.emit calls — chat
overrides handleChunk / receiveResponse / retryMessage / set messages /
getMessagesInChatCompletionsFormat with chat side effects (no no-op hooks).
Class moves / renames
- LLM (models/LLM.sys.mjs) — deleted
- MessageAccumulator (models/MessageAccumulator.sys.mjs) — replaced by
Conversation (models/Conversation.sys.mjs) - openAIEngine pulled out of models/Utils.sys.mjs into its own file
models/openAIEngine.sys.mjs; Utils re-exports it for test back-compat
with a comment pointing new callers at the new module - Remote Settings access (RS_AI_WINDOW_COLLECTION, getRemoteClient,
modelPrefObserver, _remoteClient cache) lives in models/Utils.sys.mjs;
openAIEngine is now strictly the LiteLLM-endpoint transport - MESSAGE_ROLE canonical definition lives in models/Conversation.sys.mjs;
ui/modules/ChatEnums.sys.mjs re-exports it (was previously duplicated) - PromptLoader.loadCallContext + buildLLM → buildConversation +
buildEngineForFeature helper returning {engine, parameters} for chat
(which keeps a persistent ChatConversation across turns and refreshes
engine per-turn). buildEngineForFeature resolves baseURL + apiKey via
openAIEngine.resolveEndpointConfig(modelChoiceId). - toWireFormat → getMessagesInChatCompletionsFormat on the base; chat
override applies URL token substitution and splices userContext
messages just before the last user message. The pre-refactor
getMessagesInOpenAiFormat name is gone — all callers renamed.
What moves to the base
Message: id, createdDate, ordinal, role, content, turnIndex,
parentMessageId, modelId, params, usage, toolCallId, toolName
Conversation: id, createdDate, updatedDate, feature, engine, parameters,
#messages, _minNextOrdinal, currentTurnIndex, addMessage(role, content,
turnIndex, opts), add*Message variants, setSystemMessage (idempotent),
retryMessage (generic truncate), compactChatCompletions,
getMessagesInChatCompletionsFormat, handleChunk (returns Boolean for
whether anything was extracted), receiveResponse (drain + flush, no
chat post-stream phases), run / runWithGenerator, systemPromptVersion
getter
What stays on ChatConversation
Chat-only fields: title, description, pageUrl, pageMeta, status,
securityProperties, urlToToken / tokenToUrl / #baseTokenCounts /
seenUrls, activeBranchTipMessageId, transientStarterUrl/Starters,
memoriesToggled
UI methods: renderState, addUIToolToCurrentMessage, updateToolUI; the
composed #emitter and on/off/emit forwarders
Chat orchestration:
- loadSystemPrompt(opts) — idempotent upsert. Calls loadPrompt directly
and writes body + RS-record version onto the system message content.
Called at init AND on model change. - injectRealTimeContext(message, opts) — leaf op. Owns fetch + render +
in-place mutation of message.content.userContext.realTimeContext.
Replaces static getRealTimeInfo. - injectMemoriesContext(message, prompt) — same shape for memories.
Replaces getMemoriesContext. - generatePrompt — tight sequencer: loadSystemPrompt → addUserMessage →
emit → injectRealTimeContext → injectMemoriesContext →
securityProperties.commit.
Chat overrides of base methods (call super + chat extras)
- _createMessage — factory hook; returns ChatMessage with convId set
- addAssistantMessage / addToolCallMessage — auto-populate modelId from
this.engine?.model when opts doesn't already provide it - retryMessage — captures ephemeral system messages first, then super.
Refreshes #updateActiveBranchTipMessageId() since base splices in
place (bypasses the setter) - handleChunk — calls consumeStreamChunk with this.tokenToUrl directly
(does not super through the base), then applies plainText/tokens and
emits message-update + ChatStore.updateConversation - receiveResponse — super.receiveResponse(stream, currentMessage); then
memory-id resolve, URL token strip, ChatStore.updateConversation,
message-complete emit - set messages — super + #updateActiveBranchTipMessageId
- getMessagesInChatCompletionsFormat — accepts {applyUrlTokens=true};
filters out empty-body assistant placeholders and legacy ephemeral
SYSTEM-role realtime/memories messages, splices userContext as USER
messages just before the last user message, resolves inline @mention
URLs, applies URL→token substitution via replaceUrlsWithTokens
Versioning + LLMaJ telemetry
RS-record version is NOT a Conversation field — D304293 (Bug 2044484)
landed the convention of storing version on the system message's
content (message.content.version). loadSystemPrompt writes it there.
Base Conversation exposes systemPromptVersion reading back from the
system message. TelemetryUtils.runLLMaJTelemetry loses its llm param
and reads conversation.engine?.model + conversation.systemPromptVersion
directly.
Call-site updates
- Chat.fetchWithHistory({conversation, browsingContext, mode, signal}) —
drops the llm param. Uses conversation.compactChatCompletions() +
conversation.runWithGenerator(opts). Uses conversation.engine?.model
for telemetry tool calls. - ai-window.mjs per-turn: const {engine, parameters} = await
buildEngineForFeature(MODEL_FEATURES.CHAT, opts); assigns onto the
persistent ChatConversation. Model-switch path calls
conversation.loadSystemPrompt({modelChoiceIdOverride}). - TitleGeneration / ConversationSuggestions / Memories / MemoriesManager
— use buildConversation + conversation.setSystemMessage /
addUserMessage / run(opts). Memory pipeline functions clearMessages()
between steps because they reuse one Conversation across
generation/dedup/filter. - MemoriesManager.ensureLLMForGeneration / ensureLLMForUsage →
ensureConversationForGeneration / ensureConversationForUsage.
Bug fixes uncovered during refactor
- Chat.sys.mjs addToolCallMessage call sites were passing 3 args
(content, currentTurn, toolRoleOpts) where the chat signature only
takes (content, toolOpts). toolRoleOpts was silently dropped and
currentTurn was being spread into the Message constructor as opts.
Fixed at all 5 sites; modelId is now auto-populated. - ChatConversation constructor now seeds
#updateActiveBranchTipMessageId() after super() so DB-restored
conversations have it set before the first turn (base constructor
assigned messages directly into the private array, bypassing the
chat setter).
Tests
- test_LLM.js / test_MessageAccumulator.js — deleted
- test_PromptLoader_buildLLM.js → test_PromptLoader_buildConversation.js
- test_Message.js (new), test_Conversation.js (new) — cover base classes
- test_ChatConversation.js — removed direct tests for static
getRealTimeInfo / instance getMemoriesContext; added equivalents for
injectRealTimeContext / injectMemoriesContext. - test_ChatSwitchModel.js — updateSystemPromptForModel → loadSystemPrompt
- test_Chat.js / browser_conversation_stream.js — replaced LLM
construction with a setupConversationForChat helper that assigns
conversation.engine - test_MemoriesManager.js — ensureLLMForUsage stubs renamed
- test_TelemetryUtils.js — fake conversation exposes
getMessagesInChatCompletionsFormat - browser_smartwindow_prompts.js, browser_smartwindow_retry_context.js —
stub names + arg-index assertions updated for the new instance methods - Tests that previously stubbed openAIEngine.getRemoteClient switch to a
_setRemoteClientForTesting / _clearRemoteClientForTesting test seam in
Utils.sys.mjs (openAIEngine no longer owns getRemoteClient). - Browser-test fixture ui/test/browser/head.js — uses the test seam for
the RS client and _setLoadPromptForTesting for the chat system prompt.
Test status
xpcshell: 51/51 pass.
mochitest-browser: 2354/2357 pass; the 3 failures are the
browser_smartwindow_sanitize.js suite-order flake (Bug 2006444 —
passes solo, fails in the full suite). No regressions.
End-to-end verification through ml_driver: ran user_journey happy path
on gpt-oss-120b. 14 scenarios, 45/45 turns successful, 0 inference errors,
mean LLM-judge score 3.29.
Original Revision: https://phabricator.services.mozilla.com/D302389
|
|
||
Updated•1 month ago
|
|
|
||
Updated•1 month ago
|
|
|
||
Updated•1 month ago
|
|
|
||
Updated•1 month ago
|
|
|
||
Updated•1 month ago
|
|
|
||
Updated•1 month ago
|
|
|
||
Comment 22•1 month ago
|
||
this is part of the 9 patch stack for potential exa v0 153 ride along uplifts https://phabricator.services.mozilla.com/D313875 (some abandoned for now until we get 154 beta verified)
|
|
||
Comment 23•1 month ago
|
||
marking this as fixed 154 as all but the final patch landed before 155. the specific patch from here needed for exa v0 uplift from 154 to 153 is https://hg.mozilla.org/mozilla-central/rev/bb14dccdab79
mohan, next time either convert this to a meta bug with patches in dependent bugs or split separately landing patches to followup bugs instead of leaving this open
|
|
||
Updated•1 month ago
|
|
||
Updated•1 month ago
|
|
|
||
Updated•1 month ago
|
|
||
Updated•1 month ago
|
|
|
||
Comment 24•1 month ago
|
||
| uplift | ||
Description
•