Why do we only pass along if the signature is different? What is the "signature" reflecting, really? What breaks if we always update the value - is there a performance difference?

Note that it's probably a bug how much of this code lives in - almost all of it is directly talking to model code, it's confusing that the frontend needs to care about the specifics of how the prompt is generated.

This used to assign the whole object. Why are we now assigning only the prompt, but keeping the name?

Why can't tests stub the function directly? Having a function wrapper that directly calls some other function but does nothing else is a bit smelly.

The extended commit message suggests that all this is behind a pref, but AFAICT this now unconditionally loads v2 prompts. Is the commit message wrong, or am I missing some part of how this is supposed to work, or...?

restore comment

Consider renaming to be more obvious. PROMPT_ARCHITECTURE_VERSION

Doesn't remote settings already have a local cache? Why are we doing another local cache here? Logic should be check remote settings, if prompt v2 not there, trigger a sync, check again

More caching stuff that I think is not relevant?

This feels like evals suite convention where it is openai/gpt-oss-120b, I don't think it's necessary here in MC

Why is this necessary? What model has "." in it?

This file feels like it has both prompt loading specific behavior and small helpers. Consider either organizing sections under comment blocks to help understand what is happening or even splitting file into relevant responsibilities

nit: Doesn't feel like it needs to be function

Are we worried some prompts may not be found? I think more explicit error handling here would be helpful. Why might this happen? Maybe log an error message?

Why is tableInstructions passed to this function, but format skill list is done here? Feels like there should be 1 place where we are collecting params to format?

Are we handling this error elsewhere? This name should be generated by the LLM so error handling is fine, but you are already checking later if the skill exists so not sure what this is really defending against? Either the name is valid or it's not and findSkill should tell us that?

It excludes those because our design choice in the prompt excludes them. Comment is misleading. this should include anything relevant to the prompt.

Why are we doing a backup with legacyFeatures?

Date isn't part of browser context anymore right? Because it's part of system prompt?

Where are these being set? If you delete, will they be reset at some point?

Do we need to sanitize url as well? Is this matching existing functionality?

If only relevant with chat system prompt, move into buildChatSystemPrompt

Is this to support v1? Or what is this?

nit: Is this common style in Firefox? I don't love having a special function that just makes test imports easier. I'd prefer to just import them in tests and not add extra code here

I don't think we are doing date in the browser context anymore?

Reviewing these patches, I'm wondering what the benefit of supporting v2 prompts behind a pref is? v2 is about a new architecture to load prompt in a more scalable way for the future, I'm not sure I see a purpose to allowing us to go backwards to v1 prompts as they will not be supporting (tuned, made for new models, etc.) and here it is just adding complexity to try to support both.

I would consider this a breaking change to allow only v2 prompts going forward.

I have very little context so take with a grain of salt. But, based on what you're saying, I think the only benefit is if we expect that there are situations where we want to have an emergency switch to go back to the previous version if we find breakage with v2, if we want to experiment (A/B test) between the two, or if the patch (stack) is somehow incomplete in terms of feature support, and we want to keep the v2 stuff behind a pref until it reaches parity. If none of that is the case then I agree that there probably is no real reason to keep support for the v1 version.

Done.

Done.

Done.

Done. loadV2Records calls getRemoteClient().get() directly now.

Done.

Done. Dropped the model normalization. Records hold the model identifier verbatim now.

Done, dropped together with the slash split. Records use the model identifier as is.

Done. Added section header comments organizing the file into records lookup, prompt assembly, feature config, and the public entry point.

Done. Inlined the date and template value helpers into buildChatSystemPrompt.

Left as silent skip since some modules may legitimately be absent. Happy to add a console.warn if you want a louder signal when an expected module is missing.

Done. tableInstructions, skill list, and the rest of the template values are now collected together inside buildChatSystemPrompt.

Kept the regex as a defensive guard since the LLM generates the name. Added a brief comment explaining why both checks are there.

Not sure which comment after the rebase. Most of the helper level comments got rewritten this round; let me know if a specific one is still off.

Done. Removed the legacy fallback in buildBrowserContextPrompt entirely.

Done. Date fragment removed from buildBrowserContextPrompt; date lives only in the chat browser context system prompt module.

The mapping is freshly built per call by constructRealTimeInfoInjectionMessage, so the delete only affects that one render. The next call gets a fresh mapping.

Matching existing functionality. The legacy code didn't sanitize the URL either, only the label. Can do it as a separate change if you want.

Done. Template values and skill list are inline in buildChatSystemPrompt now.

It was a v1 fallback. Removed entirely now.

Done. Removed PromptLoaderTestExports. Tests import the public functions directly and stub the RS client via head.js.

Done. Test rewritten without the date assertions.

Removed the dedupe entirely. injectRealTimeContext always writes the rendered prompt now.

Not sure I follow. Pre and post patch both assign the rendered string onto userMessage.content.userContext.realTimeContext. Happy to look closer if you can point at the line.

Done. Dropped the seam; the one test now stubs the RS client and passes a getRealTimeMapping override.

Done. Pref is gone; v2 is the only chat path now, with a soft fallback to the legacy prompts field if module records are missing.