mirror of
https://github.com/nesquena/hermes-webui.git
synced 2026-05-25 03:00:23 +00:00
Merge pull request #1643 from nesquena/stage-295
Release v0.50.295 — 3-PR batch (YAML/JSON/diff newlines + macOS scroll race + custom:* providers + glued-bold-lift raw pre)
This commit is contained in:
@@ -1,5 +1,41 @@
|
||||
# Hermes Web UI -- Changelog
|
||||
|
||||
## [v0.50.295] — 2026-05-04
|
||||
|
||||
### Fixed (3 PRs — closes #1360, #1451, #1463, #1618, #1619)
|
||||
|
||||
- **YAML, JSON, and diff/patch fenced code blocks now render multi-line, not collapsed to a single line** (#1642 by @nesquena-hermes, closes #1618 / #1463, reported by @Zixim) — PR #484 (v0.50.237) introduced a JSON/YAML tree-viewer that routes `lang === 'json'` and `lang === 'yaml'` blocks through `<div class="code-tree-wrap">…<pre class="tree-raw-view">…</pre></div>` instead of bare `<pre>`. Same release added the diff/patch coloring path that emits `<pre class="diff-block">`. The `_pre_stash` regex at `static/ui.js:1914` matched only literal `<pre>` (no attributes): `<pre>[\s\S]*?<\/pre>`. Both new shapes failed to match, fell through to the paragraph-wrap pass, and `\n` characters inside the code blocks got replaced with `<br>` tags inside `<code>`. By the time Prism ran, there were no newlines left for it to highlight against. PR #1516 (v0.50.279) had attempted a CSS-only fix on Prism's token white-space — that rule is in `style.css` and reaches the browser, but it was the wrong layer: the rule preserves newlines inside `.token` spans, but the spans were built from a string that had no newlines left. **Fix:** relax the `_pre_stash` regex to accept any attribute on `<pre>` (`<pre>` → `<pre[^>]*>`). One regex character. Pulls JSON, YAML, AND diff/patch blocks into the stash so paragraph-wrap can't mangle them. Bash, Python, Go, etc. were never affected because they emit bare `<pre>` and matched the existing regex. Reporter @Zixim noted the bug persisted from v0.50.279 → v0.50.291 → v0.50.292 despite the previous "fix"; this lands the actual fix at the actual layer.
|
||||
|
||||
> **Parallel-discovery attribution:** @Michaelyklam independently filed PR #1641 with the exact same one-character regex relax (filed 4 minutes before #1642). #1641 was closed as superseded by #1642 (which carries nesquena APPROVED + 322 LOC test suite covering YAML+JSON+diff vs #1641's YAML-only); the UI before/after PNGs from #1641 were adopted into stage-295 with a `Co-authored-by: Michael Lam` trailer on the docs commit so Michael's visual evidence ships in-tree alongside the canonical fix.
|
||||
|
||||
> **Note on the previous diagnosis:** the maintainer comment on #1618 asserting the fix had landed was based on `git show v0.50.291:static/style.css` confirming the CSS rule's presence — but a presence check on a rule is not a behavioral check that the rule does anything useful. Live-rendering YAML through `renderMd()` in the browser was the test that decided whether the maintainer reply or the user was correct. Apologies to @Zixim for the wrong call. Class of bug now documented in `webui-rendermd-pipeline` skill § Bug 10.
|
||||
|
||||
- **macOS WKWebView trackpad scroll no longer overrides user position during streaming** (#1639 by @bergeouss, closes #1360) — during streaming, scrolling up on a macOS trackpad caused the viewport to snap back to the bottom because the `_programmaticScroll setTimeout(0)` guard raced with WKWebView momentum scrolling. Mid-momentum scroll events either got swallowed (`_programmaticScroll` still True from the most recent programmatic scroll) or falsely reported nearBottom (momentum hadn't settled), keeping `_scrollPinned=true`. **Fix:** rAF-debounce the scroll listener so the nearBottom check fires on the next paint frame when the browser's scroll position has settled, plus a hysteresis counter requiring two consecutive near-bottom samples before re-pinning to prevent accidental re-pin during initial deceleration.
|
||||
|
||||
- **Custom:* providers now show all models in the dropdown** (#1639 by @bergeouss, closes #1619) — using a `custom:*` provider via `custom_providers` in `config.yaml`, the model dropdown was only showing the default model. Two parts: (1) the dedup logic in `api/config.py` ate all named-group models when they overlapped with auto-detected ones and the `continue` silently dropped auto-detected models; (2) the live enrichment endpoint at `api/routes.py:/api/models/live` only handled bare `custom`, not `custom:*` slugs. **Fix:** broadened `/api/models/live` to handle `custom:*` slugs (load-bearing fix), plus defensive belt-and-braces in `api/config.py` to fall back to auto-detected models if all named-group models were deduped (Opus advisor on stage-295 verified the latter is unreachable under current population logic but kept for future-proofing).
|
||||
|
||||
- **Glued-bold-heading lift no longer mangles raw `<pre>` HTML** (#1637 by @Michaelyklam, closes #1451) — `renderMd()` already stashed raw `<pre>` blocks before converting safe HTML tags, but restored them BEFORE the glued-bold-heading lift from #1446/#1449 ran. That left literal raw `<pre>` content visible to later markdown rewrites whenever it contained `Para text.**Heading**\n\nNext`-style text — the lift would insert `\n\n` inside the literal preformatted content, mangling it. **Fix:** delayed `rawPreStash` restore until AFTER markdown/link rewrites and BEFORE HTML sanitization. Existing placeholder pattern already protects fenced blocks; raw `<pre>` HTML now behaves like fenced code for this edge case. Test pins both sides: raw `<pre>` is preserved AND regular glued headings outside preformatted blocks still lift correctly.
|
||||
|
||||
### Tests
|
||||
|
||||
4245 → **4255 passing** (+10 regression tests across `tests/test_issue1618_yaml_json_diff_newline_preserve.py` (9), `tests/test_issue1446_glued_heading_lift.py::test_real_renderer_protects_raw_pre_html` (1); plus `tests/test_issue677.py` widened search window for #1639's rAF-debounce; plus `tests/test_745_code_block_newlines.py` widened source-scan windows from 400 to 1500 chars). 0 regressions. Full suite ~120s.
|
||||
|
||||
### Pre-release verification
|
||||
|
||||
- **Opus advisor on stage-295 combined diff: SHIP verdict.** All 6 verification questions cleared. `static/ui.js` overlap between #1637 (rawPreStash, R-token), #1639 (scroll listener), and #1642 (_pre_stash, E-token) verified non-overlapping with separate token namespaces and correct ordering. #1637's relocated restore (line 1668 → 1799) traced through every intermediate rewrite pass — placeholder `\x00R{N}\x00` has no syntactic characters that match. #1642 nested-`<pre>` non-greedy behavior verified identical to existing `rawPreStash` regex (no regression). #1639 hysteresis correct shape (count≥2 to re-pin). One non-blocking `api/config.py` defensive-dead-code observation absorbed via comment per Opus.
|
||||
- **#1642 has nesquena APPROVED** with comprehensive end-to-end behavioral trace.
|
||||
- **JS syntax**: `static/ui.js` clean.
|
||||
- **Browser API sanity**: 11/11 endpoints OK on stage server.
|
||||
- **Conflict resolution**: clean auto-merge across 3 PRs (rebased #1637 + #1639 onto current master from 9-commits-behind base).
|
||||
|
||||
### Authors
|
||||
|
||||
- @nesquena-hermes — 1 PR (#1642, with co-author trailer for @Michaelyklam's UI media adoption)
|
||||
- @Michaelyklam — 1 PR (#1637)
|
||||
- @bergeouss — 1 PR (#1639, AI-assisted via Hermes Agent)
|
||||
|
||||
Closes #1360, #1451, #1463, #1618, #1619 (5 issues).
|
||||
|
||||
## [v0.50.294] — 2026-05-04
|
||||
|
||||
### Fixed (3 PRs — streaming stability trio + models cache version stamp + session race + readonly fs guard — closes #1430, #1470, #1623, #1624, #1625, #1633)
|
||||
|
||||
+1
-1
@@ -2,7 +2,7 @@
|
||||
|
||||
> Web companion to the Hermes Agent CLI. Same workflows, browser-native.
|
||||
>
|
||||
> Last updated: v0.50.294 (May 04, 2026) — 4245 tests collected
|
||||
> Last updated: v0.50.295 (May 04, 2026) — 4255 tests collected
|
||||
> Test source: `pytest tests/ --collect-only -q`
|
||||
> Per-version detail: see [CHANGELOG.md](./CHANGELOG.md)
|
||||
|
||||
|
||||
+2
-2
@@ -1835,8 +1835,8 @@ Bridged CLI sessions:
|
||||
|
||||
---
|
||||
|
||||
*Last updated: v0.50.294, May 04, 2026*
|
||||
*Total automated tests collected: 4245*
|
||||
*Last updated: v0.50.295, May 04, 2026*
|
||||
*Total automated tests collected: 4255*
|
||||
*Regression gate: tests/test_regressions.py*
|
||||
*Run: pytest tests/ -v --timeout=60*
|
||||
*Source: <repo>/*
|
||||
|
||||
@@ -2524,6 +2524,20 @@ def get_available_models() -> dict:
|
||||
for pid in sorted(detected_providers):
|
||||
if pid.startswith("custom:") and pid in _named_custom_groups:
|
||||
_nc_display, _nc_models = _named_custom_groups[pid]
|
||||
# If all named-group models were deduped (already auto-detected
|
||||
# from base_url /v1/models), fall back to auto-detected models
|
||||
# instead of silently dropping the group (issue #1619).
|
||||
#
|
||||
# Per Opus advisor on stage-295: the load-bearing fix for the
|
||||
# reporter's symptom is the api/routes.py:/api/models/live
|
||||
# broadening to handle custom:* slugs. This block is defensive
|
||||
# belt-and-braces — under current _named_custom_groups
|
||||
# population logic (atomic add+append inside the same dedup
|
||||
# guard at line ~2640), an empty list shouldn't reach here.
|
||||
# Kept for future-proofing in case the population logic
|
||||
# changes (e.g. supporting model-less custom_providers entries).
|
||||
if not _nc_models:
|
||||
_nc_models = auto_detected_models_by_provider.get(pid, [])
|
||||
if _nc_models:
|
||||
groups.append({"provider": _nc_display, "provider_id": pid, "models": _nc_models})
|
||||
continue
|
||||
|
||||
+8
-7
@@ -4523,11 +4523,12 @@ def _handle_live_models(handler, parsed):
|
||||
ids = []
|
||||
|
||||
if not ids:
|
||||
# For 'custom' provider, provider_model_ids() returns [] because
|
||||
# 'custom' isn't a real endpoint. Fall back to the custom_providers
|
||||
# entries from config.yaml so the live-model enrichment step can
|
||||
# add any models that weren't already in the static list.
|
||||
if provider == "custom":
|
||||
# For 'custom' and 'custom:*' providers, provider_model_ids()
|
||||
# returns [] because they aren't real hermes_cli endpoints.
|
||||
# Fall back to the custom_providers entries from config.yaml so
|
||||
# the live-model enrichment step can add any models that weren't
|
||||
# already in the static list (issue #1619).
|
||||
if provider == "custom" or provider.startswith("custom:"):
|
||||
try:
|
||||
_cp_entries = cfg.get("custom_providers", [])
|
||||
if isinstance(_cp_entries, list):
|
||||
@@ -4539,8 +4540,8 @@ def _handle_live_models(handler, parsed):
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
# If still no ids, try fetching from model.base_url directly (OpenAI-compat endpoint)
|
||||
if not ids and provider == "custom":
|
||||
# If still no ids, try fetching from base_url directly (OpenAI-compat endpoint)
|
||||
if not ids and (provider == "custom" or provider.startswith("custom:")):
|
||||
_base_url = cfg.get("model", {}).get("base_url")
|
||||
_api_key = cfg.get("model", {}).get("api_key")
|
||||
if _base_url and _api_key:
|
||||
|
||||
Binary file not shown.
|
After Width: | Height: | Size: 104 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 38 KiB |
Binary file not shown.
|
After Width: | Height: | Size: 38 KiB |
+34
-10
@@ -1191,21 +1191,35 @@ window.addEventListener('resize',function(){
|
||||
// Uses a guard flag to avoid the race where programmatic scrolls (from
|
||||
// scrollIfPinned / scrollToBottom) re-set _scrollPinned=true, overriding
|
||||
// the user's explicit scroll-up. Fixes #1469 / #1360.
|
||||
// rAF-debounced scroll listener (issue #1360): on macOS WKWebView, trackpad
|
||||
// momentum scrolling fires scroll events that interleave with the
|
||||
// _programmaticScroll setTimeout(0) guard. A mid-momentum scroll event can
|
||||
// either get swallowed (_programmaticScroll still true) or falsely report
|
||||
// nearBottom (momentum hasn't settled). rAF defers the nearBottom check to
|
||||
// the next paint frame when the browser's scroll position has settled.
|
||||
// A hysteresis counter requires two consecutive near-bottom samples before
|
||||
// re-pinning, preventing accidental re-pin during initial deceleration.
|
||||
let _scrollPinned=true;
|
||||
let _programmaticScroll=false;
|
||||
let _nearBottomCount=0;
|
||||
(function(){
|
||||
const el=document.getElementById('messages');
|
||||
if(!el) return;
|
||||
let _scrollRaf=0;
|
||||
el.addEventListener('scroll',()=>{
|
||||
if(_programmaticScroll) return; // ignore scrolls we triggered ourselves
|
||||
const nearBottom=el.scrollHeight-el.scrollTop-el.clientHeight<250;
|
||||
_scrollPinned=nearBottom;
|
||||
const btn=$('scrollToBottomBtn');
|
||||
if(btn) btn.style.display=_scrollPinned?'none':'flex';
|
||||
// Load older messages when scrolled near the top
|
||||
if(el.scrollTop<80 && typeof _messagesTruncated!=='undefined' && _messagesTruncated && typeof _loadOlderMessages==='function'){
|
||||
_loadOlderMessages();
|
||||
}
|
||||
cancelAnimationFrame(_scrollRaf);
|
||||
_scrollRaf=requestAnimationFrame(()=>{
|
||||
const nearBottom=el.scrollHeight-el.scrollTop-el.clientHeight<250;
|
||||
_nearBottomCount=nearBottom?_nearBottomCount+1:0;
|
||||
_scrollPinned=_nearBottomCount>=2;
|
||||
const btn=$('scrollToBottomBtn');
|
||||
if(btn) btn.style.display=_scrollPinned?'none':'flex';
|
||||
// Load older messages when scrolled near the top
|
||||
if(el.scrollTop<80 && typeof _messagesTruncated!=='undefined' && _messagesTruncated && typeof _loadOlderMessages==='function'){
|
||||
_loadOlderMessages();
|
||||
}
|
||||
});
|
||||
});
|
||||
})();
|
||||
function _fmtTokens(n){if(!n||n<0)return'0';if(n>=1e6)return(n/1e6).toFixed(1)+'M';if(n>=1e3)return(n/1e3).toFixed(1)+'k';return String(n);}
|
||||
@@ -1665,7 +1679,6 @@ function renderMd(raw){
|
||||
s=s.replace(/<i>([\s\S]*?)<\/i>/gi,(_,t)=>'*'+t+'*');
|
||||
s=s.replace(/<code>([^<]*?)<\/code>/gi,(_,t)=>'`'+t+'`');
|
||||
s=s.replace(/<br\s*\/?>/gi,'\n');
|
||||
s=s.replace(/\x00R(\d+)\x00/g,(_,i)=>rawPreStash[+i]);
|
||||
// ── Glued-bold-heading lift (issue #1446) ────────────────────────────────
|
||||
// LLMs in thinking/reasoning mode frequently emit a "section header" glued
|
||||
// to the end of the previous paragraph with no whitespace, like:
|
||||
@@ -1797,6 +1810,9 @@ function renderMd(raw){
|
||||
s=s.replace(/(<a\b[^>]*>[\s\S]*?<\/a>)/g,m=>{_a_stash.push(m);return `\x00A${_a_stash.length-1}\x00`;});
|
||||
s=s.replace(/\[([^\]]+)\]\((https?:\/\/[^\)]+)\)/g,(_,label,url)=>`<a href="${url.replace(/"/g,'%22')}" target="_blank" rel="noopener">${esc(label)}</a>`);
|
||||
s=s.replace(/\x00A(\d+)\x00/g,(_,i)=>_a_stash[+i]);
|
||||
// Restore raw <pre> only after markdown rewrites so literal preformatted
|
||||
// content stays placeholder-protected, then let the sanitizer normalize tags.
|
||||
s=s.replace(/\x00R(\d+)\x00/g,(_,i)=>rawPreStash[+i]);
|
||||
// Sanitize any remaining HTML tags. The renderer intentionally returns
|
||||
// HTML and inserts it with innerHTML later, so tag names alone are not enough:
|
||||
// raw/model-provided HTML like <img onerror=...> or <a href="javascript:...">
|
||||
@@ -1911,7 +1927,15 @@ function renderMd(raw){
|
||||
// with <br>. Token \x00E (next free after B D F G L M C O A).
|
||||
// Fixes #745: code blocks collapse to single line when not preceded by blank line.
|
||||
const _pre_stash=[];
|
||||
s=s.replace(/(<div class="pre-header">[\s\S]*?<\/div>)?<pre>[\s\S]*?<\/pre>|<div class="(mermaid-block|katex-block)"[\s\S]*?<\/div>/g,m=>{
|
||||
// #1463 / #1618: regex must match <pre> with ANY attributes — PR #484 added
|
||||
// <pre class="tree-raw-view"> for JSON/YAML and <pre class="diff-block"> for
|
||||
// diff/patch which the literal-<pre> shape missed. Newlines inside those
|
||||
// blocks were falling through to the paragraph wrap below and getting
|
||||
// converted to <br>, causing the YAML/JSON/diff collapse. PR #1516's CSS
|
||||
// fix targeted the wrong layer (Prism token white-space) — by the time it
|
||||
// ran, the \n had already been replaced. The CSS rule is kept as defense
|
||||
// in depth.
|
||||
s=s.replace(/(<div class="pre-header">[\s\S]*?<\/div>)?<pre[^>]*>[\s\S]*?<\/pre>|<div class="(mermaid-block|katex-block)"[\s\S]*?<\/div>/g,m=>{
|
||||
_pre_stash.push(m);
|
||||
return '\x00E'+(_pre_stash.length-1)+'\x00';
|
||||
});
|
||||
|
||||
@@ -66,7 +66,7 @@ class TestCodeBlockNewlinePreservation:
|
||||
src = get_ui_js()
|
||||
# Find the replacement regex used to populate _pre_stash
|
||||
stash_block_idx = src.index('_pre_stash=[]')
|
||||
stash_block = src[stash_block_idx:stash_block_idx + 400]
|
||||
stash_block = src[stash_block_idx:stash_block_idx + 1500]
|
||||
assert 'pre-header' in stash_block, \
|
||||
"pre-stash regex must match <div class=\"pre-header\"> wrappers"
|
||||
|
||||
@@ -74,7 +74,7 @@ class TestCodeBlockNewlinePreservation:
|
||||
"""The stash regex must also cover mermaid-block divs."""
|
||||
src = get_ui_js()
|
||||
stash_block_idx = src.index('_pre_stash=[]')
|
||||
stash_block = src[stash_block_idx:stash_block_idx + 400]
|
||||
stash_block = src[stash_block_idx:stash_block_idx + 1500]
|
||||
assert 'mermaid-block' in stash_block, \
|
||||
"pre-stash regex must cover mermaid-block divs"
|
||||
|
||||
@@ -82,7 +82,7 @@ class TestCodeBlockNewlinePreservation:
|
||||
"""The stash regex must also cover katex-block divs."""
|
||||
src = get_ui_js()
|
||||
stash_block_idx = src.index('_pre_stash=[]')
|
||||
stash_block = src[stash_block_idx:stash_block_idx + 400]
|
||||
stash_block = src[stash_block_idx:stash_block_idx + 1500]
|
||||
assert 'katex-block' in stash_block, \
|
||||
"pre-stash regex must cover katex-block divs"
|
||||
|
||||
|
||||
@@ -153,20 +153,21 @@ def test_chain_of_glued_headings_all_lifted():
|
||||
|
||||
|
||||
def test_lift_pass_present_in_ui_js_at_correct_position():
|
||||
"""The lift regex must be present in ui.js, between rawPreStash restore and fence_stash restore.
|
||||
"""The lift regex must be present in ui.js before protected-code restores.
|
||||
|
||||
This pins the position so a future cleanup can't accidentally move the lift
|
||||
to a place where it would corrupt fenced code blocks (which are stashed as
|
||||
\\x00P / \\x00F tokens at this point and don't match the lift regex).
|
||||
to a place where it would corrupt raw <pre> HTML or fenced code blocks
|
||||
(which are stashed as \x00R / \x00P / \x00F tokens at this point and don't
|
||||
match the lift regex).
|
||||
"""
|
||||
lift_idx = UI_JS.find(r'(/([.!?])\*\*([^*\n]{1,80})\*\*\n\n/g')
|
||||
assert lift_idx > 0, "Glued-bold-heading lift regex not found in static/ui.js"
|
||||
raw_pre_restore = UI_JS.find("rawPreStash[+i]")
|
||||
fence_restore = UI_JS.find("fence_stash[+i]")
|
||||
assert raw_pre_restore > 0 and fence_restore > 0, "stash restore landmarks missing"
|
||||
assert raw_pre_restore < lift_idx < fence_restore, (
|
||||
"Glued-bold lift must sit between rawPreStash restore and fence_stash restore "
|
||||
"so fenced code is protected. Current ordering broken."
|
||||
assert lift_idx < raw_pre_restore and lift_idx < fence_restore, (
|
||||
"Glued-bold lift must run before rawPreStash and fence_stash restore "
|
||||
"so raw <pre> and fenced code are protected. Current ordering broken."
|
||||
)
|
||||
|
||||
|
||||
@@ -254,6 +255,16 @@ def test_real_renderer_protects_fenced_code(driver_path):
|
||||
assert "**inside-code**" in out, out
|
||||
|
||||
|
||||
@pytest.mark.skipif(NODE is None, reason="node not on PATH")
|
||||
def test_real_renderer_protects_raw_pre_html(driver_path):
|
||||
"""Raw literal <pre> content must stay byte-preserved when it contains the glued trigger."""
|
||||
src = "<pre>Para text.**Heading**\n\nNext.</pre>\n"
|
||||
out = _render(driver_path, src)
|
||||
assert "<pre>Para text.**Heading**\n\nNext.</pre>" in out, out
|
||||
assert "<pre>Para text.\n\n**Heading**\n\nNext.</pre>" not in out, out
|
||||
assert "<strong>Heading</strong>" not in out, out
|
||||
|
||||
|
||||
@pytest.mark.skipif(NODE is None, reason="node not on PATH")
|
||||
def test_real_renderer_protects_inline_code(driver_path):
|
||||
"""Glued pattern inside inline backticks must stay literal."""
|
||||
|
||||
@@ -0,0 +1,322 @@
|
||||
"""Tests for issue #1618 / #1463 — YAML/JSON code blocks render flattened.
|
||||
|
||||
Bug shape (live-verified in the browser May 04 2026):
|
||||
|
||||
```yaml
|
||||
foo:
|
||||
bar: 1
|
||||
baz:
|
||||
```
|
||||
|
||||
renders as a single line `foo: bar: 1 baz:` with no newlines, while:
|
||||
|
||||
```yml
|
||||
foo:
|
||||
bar: 1
|
||||
baz:
|
||||
```
|
||||
|
||||
renders correctly multi-line. PR #1516 (v0.50.279) shipped a CSS-only fix
|
||||
targeting Prism token white-space; the rule is in `style.css` and reaches
|
||||
the browser, but the bug persists because the actual newline destruction
|
||||
happens earlier in the pipeline, before Prism runs.
|
||||
|
||||
Root cause:
|
||||
- PR #484 (v0.50.237, JSON/YAML tree-viewer) routes those two languages
|
||||
through `<div class="code-tree-wrap">…<pre class="tree-raw-view">`
|
||||
instead of bare `<pre>`.
|
||||
- The `_pre_stash` regex at static/ui.js:1914 matched only literal `<pre>`
|
||||
with NO attributes (`<pre>[\\s\\S]*?<\\/pre>`).
|
||||
- `<pre class="tree-raw-view">` doesn't match → falls through to the
|
||||
paragraph wrap pass which replaces `\\n` with `<br>`.
|
||||
- By the time Prism runs and the CSS rule applies, the `\\n` characters
|
||||
that the rule was meant to preserve are already gone.
|
||||
|
||||
Same bug affects:
|
||||
- `lang === 'yaml'` (issue #1463 / #1618 — the canonical case)
|
||||
- `lang === 'json'` (same code path at static/ui.js:1621)
|
||||
- `lang === 'diff'` / `lang === 'patch'` (`<pre class="diff-block">`,
|
||||
same shape, same regex miss — emits at static/ui.js:1619)
|
||||
|
||||
Fix: relax the `_pre_stash` regex to accept any attribute on `<pre>`:
|
||||
`<pre>[\\s\\S]*?<\\/pre>` → `<pre[^>]*>[\\s\\S]*?<\\/pre>`
|
||||
|
||||
These tests pin both the source-level invariant (regex shape) and the
|
||||
end-to-end behavior via a node-driver that exercises the actual
|
||||
static/ui.js renderMd() function.
|
||||
"""
|
||||
|
||||
import shutil
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
REPO_ROOT = Path(__file__).parent.parent.resolve()
|
||||
UI_JS_PATH = REPO_ROOT / "static" / "ui.js"
|
||||
NODE = shutil.which("node")
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────
|
||||
# § A — Source-string invariants (run without node, fast)
|
||||
# ─────────────────────────────────────────────────────────────────────────
|
||||
|
||||
|
||||
def test_pre_stash_regex_matches_pre_with_attributes():
|
||||
"""static/ui.js _pre_stash regex must match <pre> with ANY attributes.
|
||||
|
||||
The narrow shape `<pre>[\\s\\S]*?<\\/pre>` (literal <pre> with no
|
||||
attributes) misses every <pre class="..."> emitted by the JSON/YAML
|
||||
tree-viewer pass and the diff/patch coloring pass — those blocks fall
|
||||
through to paragraph wrap, which converts \\n to <br>.
|
||||
"""
|
||||
src = UI_JS_PATH.read_text(encoding="utf-8")
|
||||
|
||||
# The fix introduces `<pre[^>]*>` (any attributes) in the _pre_stash regex.
|
||||
# The exact regex line is documented in static/ui.js:1914.
|
||||
assert "<pre[^>]*>[\\s\\S]*?<\\/pre>" in src, (
|
||||
"_pre_stash regex must use <pre[^>]*> to match <pre> with any attributes "
|
||||
"(#1463/#1618). The narrow shape <pre>[\\s\\S]*?<\\/pre> misses every "
|
||||
"<pre class=\"tree-raw-view\"> from the JSON/YAML tree-viewer (PR #484) "
|
||||
"and <pre class=\"diff-block\"> from diff/patch — newlines inside those "
|
||||
"blocks fall through to paragraph wrap and become <br> tags."
|
||||
)
|
||||
|
||||
# Defense against accidental regression: the literal-only shape must NOT
|
||||
# be present anywhere in the _pre_stash region of the file.
|
||||
pre_stash_idx = src.find("const _pre_stash=[]")
|
||||
assert pre_stash_idx > 0, "_pre_stash declaration not found"
|
||||
pre_stash_line = src[pre_stash_idx:pre_stash_idx + 1500]
|
||||
assert "<pre>[\\s\\S]*?<\\/pre>" not in pre_stash_line, (
|
||||
"_pre_stash regex must not contain the literal-<pre>-only shape — "
|
||||
"use <pre[^>]*> to match attributes."
|
||||
)
|
||||
|
||||
|
||||
def test_pre_stash_still_captures_pre_header_and_optional_div():
|
||||
"""The fix must keep the rest of the _pre_stash regex intact —
|
||||
specifically the optional <div class="pre-header"> prefix and the
|
||||
mermaid-block / katex-block alternation."""
|
||||
src = UI_JS_PATH.read_text(encoding="utf-8")
|
||||
|
||||
pre_stash_idx = src.find("const _pre_stash=[]")
|
||||
pre_stash_block = src[pre_stash_idx:pre_stash_idx + 1500]
|
||||
|
||||
assert '(<div class="pre-header">[\\s\\S]*?<\\/div>)?<pre[^>]*>' in pre_stash_block, (
|
||||
"Optional <div class=\"pre-header\"> prefix must still precede the "
|
||||
"<pre[^>]*> match"
|
||||
)
|
||||
assert '<div class="(mermaid-block|katex-block)"' in pre_stash_block, (
|
||||
"Mermaid/katex block alternation must remain in the regex"
|
||||
)
|
||||
|
||||
|
||||
# ─────────────────────────────────────────────────────────────────────────
|
||||
# § B — Behavioural tests via node-driver (skipped if node not on PATH)
|
||||
# ─────────────────────────────────────────────────────────────────────────
|
||||
|
||||
pytestmark_node = pytest.mark.skipif(NODE is None, reason="node not on PATH")
|
||||
|
||||
|
||||
# Reuses the same driver shape as tests/test_renderer_js_behaviour.py.
|
||||
_DRIVER_SRC = r"""
|
||||
const fs = require('fs');
|
||||
const src = fs.readFileSync(process.argv[2], 'utf8');
|
||||
global.window = {};
|
||||
global.document = { createElement: () => ({ innerHTML: '', textContent: '' }) };
|
||||
const esc = s => String(s ?? '').replace(/[&<>"']/g, c => (
|
||||
{'&':'&','<':'<','>':'>','"':'"',"'":'''}[c]));
|
||||
const _IMAGE_EXTS=/\.(png|jpg|jpeg|gif|webp|bmp|ico|avif)$/i;
|
||||
const _SVG_EXTS=/\.svg$/i;
|
||||
const _AUDIO_EXTS=/\.(mp3|ogg|wav|m4a|aac|flac|wma|opus|webm)$/i;
|
||||
const _VIDEO_EXTS=/\.(mp4|webm|mkv|mov|avi|ogv|m4v)$/i;
|
||||
|
||||
function extractFunc(name) {
|
||||
const re = new RegExp('function\\s+' + name + '\\s*\\(');
|
||||
const start = src.search(re);
|
||||
if (start < 0) throw new Error(name + ' not found');
|
||||
let i = src.indexOf('{', start);
|
||||
let depth = 1; i++;
|
||||
while (depth > 0 && i < src.length) {
|
||||
if (src[i] === '{') depth++;
|
||||
else if (src[i] === '}') depth--;
|
||||
i++;
|
||||
}
|
||||
return src.slice(start, i);
|
||||
}
|
||||
eval(extractFunc('renderMd'));
|
||||
|
||||
let buf = '';
|
||||
process.stdin.on('data', c => { buf += c; });
|
||||
process.stdin.on('end', () => { process.stdout.write(renderMd(buf)); });
|
||||
"""
|
||||
|
||||
|
||||
@pytest.fixture(scope="module")
|
||||
def driver_path(tmp_path_factory):
|
||||
p = tmp_path_factory.mktemp("issue1618_driver") / "driver.js"
|
||||
p.write_text(_DRIVER_SRC, encoding="utf-8")
|
||||
return str(p)
|
||||
|
||||
|
||||
def _render(driver_path, markdown: str) -> str:
|
||||
"""Run renderMd against the actual ui.js and return the rendered HTML."""
|
||||
result = subprocess.run(
|
||||
[NODE, driver_path, str(UI_JS_PATH)],
|
||||
input=markdown,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=10,
|
||||
)
|
||||
if result.returncode != 0:
|
||||
raise RuntimeError(f"node driver failed: {result.stderr}")
|
||||
return result.stdout
|
||||
|
||||
|
||||
def _extract_pre_inner(html: str) -> str:
|
||||
"""Extract the content of the first <pre ...>...</pre> block."""
|
||||
import re
|
||||
m = re.search(r"<pre[^>]*>([\s\S]*?)</pre>", html)
|
||||
if not m:
|
||||
return ""
|
||||
return m.group(1)
|
||||
|
||||
|
||||
# ── The core regression: YAML newlines must survive ────────────────────
|
||||
|
||||
|
||||
@pytestmark_node
|
||||
def test_yaml_block_preserves_newlines(driver_path):
|
||||
"""YAML code blocks must render multi-line, not flatten to a single line.
|
||||
|
||||
This is the exact symptom Zixim reported on #1618: a YAML block renders
|
||||
with all newlines collapsed to spaces. The fix is the relaxed _pre_stash
|
||||
regex; without it, the block falls through to paragraph wrap and \\n
|
||||
becomes <br> inside <code>, which Prism then can't recover from.
|
||||
"""
|
||||
md = "```yaml\nfoo:\n bar: 1\n baz:\n - 2\n - 3\n```"
|
||||
out = _render(driver_path, md)
|
||||
|
||||
# The block must end up wrapped in code-tree-wrap (PR #484's shape)
|
||||
assert "code-tree-wrap" in out, (
|
||||
"YAML blocks should still route through the tree-viewer wrapper"
|
||||
)
|
||||
|
||||
# Inner <pre>...</pre> must contain literal \n characters (preserved
|
||||
# newlines), NOT <br> tags.
|
||||
pre_inner = _extract_pre_inner(out)
|
||||
assert pre_inner, f"No <pre> block found in rendered output: {out!r}"
|
||||
assert "\n" in pre_inner, (
|
||||
f"YAML <pre> block lost its newlines (#1463/#1618). "
|
||||
f"<pre> inner content: {pre_inner!r}. "
|
||||
f"Likely cause: _pre_stash regex doesn't match <pre class=\"tree-raw-view\">, "
|
||||
f"so the block falls through to the paragraph wrap pass which converts \\n to <br>."
|
||||
)
|
||||
assert "<br>" not in pre_inner, (
|
||||
f"YAML <pre> block contains <br> tags — newlines were converted by paragraph "
|
||||
f"wrap. This means the _pre_stash regex did not capture the block. "
|
||||
f"<pre> inner content: {pre_inner!r}"
|
||||
)
|
||||
|
||||
|
||||
@pytestmark_node
|
||||
def test_json_block_preserves_newlines(driver_path):
|
||||
"""JSON code blocks have the same shape as YAML (PR #484) and must also
|
||||
preserve newlines."""
|
||||
md = '```json\n{\n "a": 1,\n "b": [2, 3]\n}\n```'
|
||||
out = _render(driver_path, md)
|
||||
|
||||
assert "code-tree-wrap" in out
|
||||
pre_inner = _extract_pre_inner(out)
|
||||
assert pre_inner
|
||||
assert "\n" in pre_inner, (
|
||||
f"JSON <pre> block lost newlines. Inner: {pre_inner!r}"
|
||||
)
|
||||
assert "<br>" not in pre_inner
|
||||
|
||||
|
||||
@pytestmark_node
|
||||
def test_diff_block_preserves_newlines(driver_path):
|
||||
"""Diff/patch blocks emit <pre class=\"diff-block\"> (static/ui.js:1619).
|
||||
Same regex-miss shape as YAML/JSON. Newlines must survive."""
|
||||
md = "```diff\n-removed line\n+added line\n unchanged\n```"
|
||||
out = _render(driver_path, md)
|
||||
|
||||
assert "diff-block" in out
|
||||
pre_inner = _extract_pre_inner(out)
|
||||
assert pre_inner
|
||||
assert "\n" in pre_inner, (
|
||||
f"Diff <pre> block lost newlines. Inner: {pre_inner!r}"
|
||||
)
|
||||
assert "<br>" not in pre_inner
|
||||
|
||||
|
||||
@pytestmark_node
|
||||
def test_yml_alias_already_worked_still_works(driver_path):
|
||||
"""Sanity check: ` ```yml ` (the Prism alias) renders bare <pre> and
|
||||
was never affected by the bug. This must continue to work after the
|
||||
regex relaxation."""
|
||||
md = "```yml\nfoo:\n bar: 1\n```"
|
||||
out = _render(driver_path, md)
|
||||
pre_inner = _extract_pre_inner(out)
|
||||
assert "\n" in pre_inner
|
||||
assert "<br>" not in pre_inner
|
||||
|
||||
|
||||
@pytestmark_node
|
||||
def test_bash_block_unaffected_baseline(driver_path):
|
||||
"""Sanity: bash blocks emit bare <pre> and were never affected by the bug.
|
||||
They must continue to render correctly post-fix."""
|
||||
md = "```bash\necho one\necho two\n```"
|
||||
out = _render(driver_path, md)
|
||||
pre_inner = _extract_pre_inner(out)
|
||||
assert "\n" in pre_inner
|
||||
assert "<br>" not in pre_inner
|
||||
|
||||
|
||||
# ── End-to-end Zixim-scenario reproducer ───────────────────────────────
|
||||
|
||||
|
||||
@pytestmark_node
|
||||
def test_yaml_block_renders_multiline_html_shape(driver_path):
|
||||
"""The specific shape Zixim reported: 5-line YAML block must produce
|
||||
exactly 5 newline-separated logical lines in the <pre> inner content.
|
||||
|
||||
Pre-fix this collapsed to a single space-joined string. Post-fix the
|
||||
line count should equal the original input line count.
|
||||
"""
|
||||
md = "```yaml\nname: hermes\nport: 8787\nfeatures:\n - chat\n - tasks\n```"
|
||||
out = _render(driver_path, md)
|
||||
|
||||
pre_inner = _extract_pre_inner(out)
|
||||
# Split on \n to count rendered lines. Empty trailing line tolerated.
|
||||
rendered_lines = [l for l in pre_inner.split("\n") if l.strip()]
|
||||
|
||||
assert len(rendered_lines) == 5, (
|
||||
f"YAML block should preserve 5 lines, got {len(rendered_lines)}: {rendered_lines}. "
|
||||
f"Full <pre> inner content: {pre_inner!r}"
|
||||
)
|
||||
|
||||
|
||||
# ── Mermaid/katex blocks unaffected ────────────────────────────────────
|
||||
|
||||
|
||||
@pytestmark_node
|
||||
def test_mermaid_block_unaffected_by_regex_relaxation(driver_path):
|
||||
"""Mermaid blocks come through a different alternation in the same regex
|
||||
(`<div class=\"(mermaid-block|katex-block)\"...`). Confirm they still get
|
||||
captured into _pre_stash and aren't paragraph-wrapped."""
|
||||
md = "```mermaid\ngraph TD\n A --> B\n B --> C\n```"
|
||||
out = _render(driver_path, md)
|
||||
|
||||
# Mermaid block emits <div class="mermaid-block"> (no <pre>).
|
||||
assert "mermaid-block" in out
|
||||
# The mermaid div should not be wrapped in <p>...</p>.
|
||||
assert "<p><div class=\"mermaid-block\"" not in out
|
||||
# Internal newlines inside data-mermaid-id should not be relevant —
|
||||
# mermaid content is in the data-attr / esc()'d innerText. But the
|
||||
# surrounding paragraph-wrap-bypass MUST still work.
|
||||
assert "<p>" not in out or out.find("<p>") > out.find("mermaid-block"), (
|
||||
"Mermaid block should bypass paragraph wrap"
|
||||
)
|
||||
@@ -120,7 +120,9 @@ class TestScrollPinningFix:
|
||||
"""Scroll listener must hide the button when user is near the bottom (#677)."""
|
||||
scroll_listener_start = UI_JS.find("el.addEventListener('scroll'")
|
||||
assert scroll_listener_start != -1, "scroll event listener not found"
|
||||
listener_block = UI_JS[scroll_listener_start:scroll_listener_start + 300]
|
||||
# After #1360 fix, the nearBottom + btn logic lives inside an rAF
|
||||
# callback — extend search window to cover the full listener block.
|
||||
listener_block = UI_JS[scroll_listener_start:scroll_listener_start + 600]
|
||||
assert "scrollToBottomBtn" in listener_block, (
|
||||
"Scroll listener must show/hide scrollToBottomBtn based on _scrollPinned (#677)"
|
||||
)
|
||||
|
||||
@@ -69,9 +69,9 @@ def render_md(raw):
|
||||
s = re.sub(r"<i>([\s\S]*?)</i>", lambda m: "*" + m.group(1) + "*", s, flags=re.I)
|
||||
s = re.sub(r"<code>([^<]*?)</code>", lambda m: "`" + m.group(1) + "`", s, flags=re.I)
|
||||
s = re.sub(r"<br\s*/?>", "\n", s, flags=re.I)
|
||||
# Glued-bold-heading lift (issue #1446) — must mirror static/ui.js position:
|
||||
# after raw <pre> restore, before fence_stash restore. Lifts a sentence-glued
|
||||
# bold "stub heading" out into its own paragraph when followed by a blank line.
|
||||
# Glued-bold-heading lift (issue #1446) — must mirror static/ui.js behavior:
|
||||
# protected code/pre placeholders stay hidden while a sentence-glued bold
|
||||
# "stub heading" is lifted into its own paragraph when followed by a blank line.
|
||||
s = re.sub(r"([.!?])\*\*([^*\n]{1,80})\*\*\n\n", r"\1\n\n**\2**\n\n", s)
|
||||
s = re.sub(r"\x00F(\d+)\x00", lambda m: fence_stash[int(m.group(1))], s)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user