ChatRenderer is overloaded - performing heavy lifting outside of rendering chats.

[entitycs]

Overview

Just as the title says, this is a great module on its own - but it is increasingly being overworked, and over-referenced externally, unnecessarily, and against maintainable / best principles.

I hope I do not have to list all of the reasons this file needs refactoring...

Said shortly, we can get more out of it (and out of what it contains) by separating concerns a bit more than what exists currently.

The current list of exports contains components that ChatRenderer should not be tasked with providing.

The Current Refactor

The current state of the refactor merge request.

Models

What the refactor pulls out of ChatRenderer, and puts into the existing model folder

graph LR
    A[Model Extractions] --> B[Data Layer]
    A --> C[Helper Functions]
    A --> D[Endpoint Detection]
    A --> E[Cost Calculation]
    
    B --> B1[MODEL_INFO<br/>Pricing & Context]
    B --> B2[IMAGE_PRICING<br/>Image Costs]
    B --> B3[MODEL_PRICING<br/>Compat Alias]
    
    C --> C1[shortModel<br/>Name simplification]
    C --> C2[sameModelName<br/>Name comparison]
    C --> C3[modelRouteLabel<br/>Display labels]
    C --> C4[replyModelPair<br/>Extract metadata]
    C --> C5[modelColor<br/>HSL generation]
    C --> C6[getModelInfo<br/>Lookup pricing]
    
    D --> D1[isLocalEndpoint<br/>Free models]
    D --> D2[isSubscriptionEndpoint<br/>ChatGPT]
    D --> D3[isCostTrackedEndpoint<br/>Billable]
    
    E --> E1[getModelCost<br/>Calculate textual cost]
    E --> E2[getImageCost<br/>Calculate image cost]

    style A fill:#e1f5ff,color:black
    style B fill:#fff4e1,color:black
    style C fill:#f3e5f5,color:black
    style D fill:#e8f5e9,color:black
    style E fill:#ffebee,color:black

Loading

Utility functions (Safe Strings)

Next up to be extracted from this file, are some useful UISafeString functions:

/** Sanitize a URL for use in href — only allow http(s) and protocol-relative. */
function _safeHref(url) {
  if (!url) return '#';
  try {
    var parsed = new URL(url, window.location.origin);
    if (parsed.protocol === 'http:' || parsed.protocol === 'https:') return uiModule.esc(url);
  } catch (e) { /* invalid URL */ }
  return '#';
}

export function safeToolScreenshotSrc(raw) {
  const src = String(raw || '').trim();
  if (/^data:image\/(?:png|jpe?g|gif|webp);base64,[a-z0-9+/=\s]+$/i.test(src)) {
    return src;
  }
  return '';
}

export function safeDisplayImageSrc(raw) {
  const src = String(raw || '').trim();
  if (!src) return '';
  if (/^data:image\/(?:png|jpe?g|gif|webp);base64,[a-z0-9+/=\s]+$/i.test(src)) {
    return src;
  }
  try {
    const parsed = new URL(src, window.location.origin);
    if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
      return parsed.href;
    }
  } catch (_) { }
  return '';
}

Utility functions ((string) Format)

• static/js/chat/format: formatCompactNumber/toTrimmedString utilities (previously chatRenderer)
  • formatCompactNumber existed as _fmtCtx and may be useful outside of chatRenderer.
  • toTrimmedString existed as modelValue and is now much more clear as to its function, plus may be useful outside of chatRenderer.

Benefits:

Code maintainability is improved because pricing data, validation helpers, and model-related exports have a source of truth outside of the overworked renderer file. Future changes become isolated and easier to reason about.

~300 fewer lines to sift through (it's a start)

Test Plan:

• Verify all imports resolve correctly
• Reaffirm logic - render-related exports remain in chatRenderer.js
• Reaffirm logic - If a module is importing chatRenderer.js, it should be because it needs information about the rendering of chats - not the pricing of models, for example.

[alteixeira20]

This looks like a good concrete slice of the broader architecture/readability cleanup.

The screenshots make the boundary problem pretty clear: ChatRenderer is not only rendering chats. It is also acting as a source for model metadata, model-name helpers, endpoint detection, pricing/cost helpers, safe image/string utilities, session-cost helpers, and rendering functions. That makes the module harder to reason about and also encourages unrelated code to import from a renderer just to access non-rendering helpers.

I especially like that this proposed extraction is not trying to rewrite the frontend stack or change chat behavior. Pulling model metadata, pricing/cost helpers, endpoint detection, formatting helpers, and safe URL/image helpers out of ChatRenderer is the right kind of first step: narrow, behavior-preserving, and directly tied to one overloaded responsibility boundary.

The line I would hold is:

ChatRenderer should only be imported when a caller needs chat-rendering behavior.

If a module needs model pricing, endpoint labels, cost calculation, safe image source validation, or formatting utilities, those should live in focused modules with names that describe their actual responsibility.

I would keep the first PR intentionally small:

• extract model metadata/pricing helpers into a model-focused module;
• extract endpoint detection into a model/provider utility module;
• extract safe URL/image/string helpers into a rendering-safety or UI-safe-string module;
• leave actual chat rendering behavior unchanged;
• keep temporary compatibility exports if that makes the migration safer;
• add focused tests for the extracted helpers;
• verify current chat rendering still behaves the same.

This would be a strong example of the kind of incremental refactor Odysseus needs: not a rewrite, not a framework decision, just a clear responsibility boundary with a reviewable diff. If this pattern works well, it can become the template for extracting other overloaded frontend modules.

[entitycs]

Thanks for the feedback. I will try to keep it updated for now, through rebasing while validating each bullet point.

Some of the extracted modules (eg. safeString/*) already had tests; however, they were performed through chatRenderer. I addressed the tests needed to pass as is, but did not add any tests. I suppose since behavior doesn't change, adding further test coverage was ?optional?

One possible overstep that I noticed during self-review is the validate_imports test helper function. I do believe it's valuable, in specifically addressing the needed changes / additions to the test suite after refactor (in such a way that they will not break if/when linting is applied). But if added in its own commit, it could more easily be extracted out, patched, etc.

[entitycs]

The "it" being updated: #4540 refactor(chatRenderer): Refactor chatRenderer.js such that it only contains data, exports, and logic related to rendering chats.

Self-reviewed once or twice, with the lasting critique being the 2nd commit required to fix an accidental save-from-ide that linted one of the files.

[entitycs]

Cleaned it up so that it includes only a single commit + adheres to @pewdiepie-archdaemon request for change, with concise comment

[entitycs]

Recalling SafeDisplayImageSrc became a util function, I found a "second source of truth" for this string, defined in static/js/notes.js:

It also has tests (two things in common with chatRenderer).

definition from notes:

// Image src guard — reject anything that isn't a relative path, http(s), or
// raster data URL so an AI-saved note can't slip script-capable media into the
// rendered <img>.
function _safeImgSrc(s) {
  const v = (s || '').trim();
  if (!v) return '';
  if (v.startsWith('/') || v.startsWith('./') || v.startsWith('../')) return v;
  if (/^https?:\/\//i.test(v) || /^data:image\/(?:png|jpe?g|gif|webp);base64,/i.test(v)) return v;
  return '';
}

vs what we pulled from ChatRenderer in this PR

export function safeDisplayImageSrc(raw) {
  const src = String(raw || '').trim();
  if (!src) return '';
  if (/^data:image\/(?:png|jpe?g|gif|webp);base64,[a-z0-9+/=\s]+$/i.test(src)) {
    return src;
  }
  try {
    const parsed = new URL(src, window.location.origin);
    if (parsed.protocol === 'http:' || parsed.protocol === 'https:') {
      return parsed.href;
    }
  } catch {/* Silent fail */}
  return '';
}

The latter is more robust, so locally, I just wiped the definition found in notes.js and imported SafeDisplayImageSrc from static/js/util/safeString.js.

P.S. Antigravity suggests some slight improvements, and chose the latter as its base with good reasoning - but this is just a refactor, so, leaving a memo.

I suppose I'll hold back on including the suggested changes to notes.js. It could use another PR with a source-of-truth 'label'.