diff --git a/api/streaming.py b/api/streaming.py index c861f01..6bd7201 100644 --- a/api/streaming.py +++ b/api/streaming.py @@ -74,6 +74,25 @@ def _strip_thinking_markup(text: str) -> str: return s +def _strip_xml_tool_calls(text: str) -> str: + """Strip XML-style function_calls blocks that DeepSeek and similar models + emit in their raw response text. These blocks are processed separately as + tool calls; leaving them in the assistant content causes them to render + visibly in the chat bubble. + + Handles both complete blocks (…) and + partial/orphaned opening tags that may appear at the tail of a stream. + """ + if not text or '' not in text.lower(): + return text + s = str(text) + # Strip complete blocks (possibly multiple) + s = re.sub(r'.*?', '', s, flags=re.IGNORECASE | re.DOTALL) + # Strip orphaned opening tags (stream cut off before closing tag) + s = re.sub(r'.*$', '', s, flags=re.IGNORECASE | re.DOTALL) + return s.strip() + + def _sanitize_generated_title(text: str) -> str: """Sanitize LLM-generated title text before persisting to session.""" s = _strip_thinking_markup(text or '') @@ -1137,6 +1156,21 @@ def _run_agent_streaming(session_id, msg_text, model, workspace, stream_id, atta _previous_messages, result.get('messages') or s.messages, ) + # Strip XML tool-call blocks from assistant message content. + # DeepSeek and some other providers emit ... + # in the raw response text; this must be removed before the content is + # saved to the session and displayed in the chat bubble. (#702) + for _m in s.messages: + if isinstance(_m, dict) and _m.get('role') == 'assistant': + _raw_content = _m.get('content') + if isinstance(_raw_content, str): + _cleaned = _strip_xml_tool_calls(_raw_content) + if _cleaned != _raw_content: + _m['content'] = _cleaned + elif isinstance(_raw_content, list): + for _part in _raw_content: + if isinstance(_part, dict) and isinstance(_part.get('text'), str): + _part['text'] = _strip_xml_tool_calls(_part['text']) # ── Detect silent agent failure (no assistant reply produced) ── # When the agent catches an auth/network error internally it may return diff --git a/static/i18n.js b/static/i18n.js index f888e22..a6bb51c 100644 --- a/static/i18n.js +++ b/static/i18n.js @@ -147,6 +147,8 @@ const LOCALES = { failed_colon: 'Failed: ', // ui.js no_workspace: 'No workspace', + workspace_empty_no_path: 'No workspace selected. Set a workspace in Settings \u2192 Workspace to browse files.', + workspace_empty_dir: 'This workspace is empty.', dialog_confirm_title: 'Confirm action', dialog_prompt_title: 'Enter a value', dialog_confirm_btn: 'Confirm', @@ -255,7 +257,7 @@ const LOCALES = { settings_label_sound: 'Notification sound', settings_desc_sound: 'Play a sound when the assistant finishes a response.', settings_label_notifications: 'Browser notifications', - settings_desc_notifications: 'Show a system notification when a response completes while the tab is in the background.', + settings_desc_notifications: 'Show a system notification when a response completes while the app is in the background.', settings_desc_token_usage: 'Displays input/output token count below each assistant reply. Also toggled with /usage.', settings_desc_bubble_layout: 'Right-align user messages and left-align assistant replies. Off by default to keep code blocks and tool output full-width.', settings_desc_cli_sessions: 'Merges sessions from the Hermes CLI (state.db) into the session list. Click a CLI session to import it and continue the conversation.', @@ -572,6 +574,8 @@ const LOCALES = { failed_colon: 'Error: ', // ui.js no_workspace: 'Sin espacio de trabajo', + workspace_empty_no_path: 'No hay espacio de trabajo seleccionado. Configure un espacio de trabajo en Ajustes \u2192 Workspace para explorar archivos.', + workspace_empty_dir: 'Este espacio de trabajo está vacío.', // workspace.js unsaved_confirm: 'Tienes cambios sin guardar en la vista previa. ¿Descartar y navegar?', save: 'Guardar', @@ -985,6 +989,8 @@ const LOCALES = { failed_colon: 'Fehlgeschlagen: ', // ui.js no_workspace: 'Kein Workspace', + workspace_empty_no_path: 'Kein Workspace ausgewählt. Wähle einen Workspace unter Einstellungen \u2192 Workspace, um Dateien zu durchsuchen.', + workspace_empty_dir: 'Dieser Workspace ist leer.', dialog_confirm_title: 'Aktion bestätigen', dialog_prompt_title: 'Wert eingeben', dialog_confirm_btn: 'Bestätigen', @@ -1199,6 +1205,9 @@ const LOCALES = { theme_usage: '\u7528\u6cd5\uff1a/theme ', theme_set: '\u4e3b\u9898\uff1a', no_active_session: '\u5f53\u524d\u6ca1\u6709\u6d3b\u52a8\u4f1a\u8bdd', + + workspace_empty_no_path: '未选择工作区。请在 设置 → 工作区 中设置工作区以浏览文件。', + workspace_empty_dir: '此工作区为空。', no_personalities: '\u6ca1\u6709\u627e\u5230\u4eba\u8bbe\uff08\u53ef\u6dfb\u52a0\u5230 ~/.hermes/personalities/\uff09', available_personalities: '\u53ef\u7528\u4eba\u8bbe\uff1a', personality_switch_hint: '\n\n\u4f7f\u7528 `/personality ` \u5207\u6362\uff0c\u6216\u7528 `/personality none` \u6e05\u7a7a\u3002', @@ -1611,6 +1620,9 @@ const LOCALES = { theme_usage: '\u7528\u6cd5\uff1a/theme ', theme_set: '\u4e3b\u984c\uff1a', no_active_session: '\u7576\u524d\u6c92\u6709\u6d3b\u52d5\u6703\u8a71', + + workspace_empty_no_path: '未選擇工作區。請在 設定 → 工作區 中設定工作區以瀏覽檔案。', + workspace_empty_dir: '此工作區為空。', no_personalities: '\u6c92\u6709\u627e\u5230\u4eba\u8a2d\uff08\u53ef\u6dfb\u52a0\u5230 ~/.hermes/personalities/\uff09', available_personalities: '\u53ef\u7528\u4eba\u8a2d\uff1a', personality_switch_hint: '\n\n\u4f7f\u7528 `/personality ` \u5207\u63db\uff0c\u6216\u7528 `/personality none` \u6e05\u7a7a\u3002', diff --git a/static/index.html b/static/index.html index 620df4d..6877e70 100644 --- a/static/index.html +++ b/static/index.html @@ -390,6 +390,7 @@
+
diff --git a/static/messages.js b/static/messages.js index fb5d4f6..3c3597a 100644 --- a/static/messages.js +++ b/static/messages.js @@ -229,8 +229,17 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ let _renderPending=false; // Extract display text from assistantText, stripping completed thinking blocks // and hiding content still inside an open thinking block. + function _stripXmlToolCalls(s){ + // Strip ... blocks (DeepSeek XML tool syntax). + // These are processed as tool calls server-side; showing them raw in the bubble + // looks broken. Also handles orphaned opening tags mid-stream. (#702) + if(!s||s.toLowerCase().indexOf('')===-1) return s; + s=s.replace(/[\s\S]*?<\/function_calls>/gi,''); + s=s.replace(/[\s\S]*$/i,''); + return s.trim(); + } function _streamDisplay(){ - const raw=assistantText; + const raw=_stripXmlToolCalls(assistantText); if(reasoningText) return raw; for(const {open,close} of _thinkPairs){ // Trim leading whitespace before checking for the open tag — some models @@ -252,7 +261,7 @@ function attachLiveStream(activeSid, streamId, uploaded=[], options={}){ return raw; } function _parseStreamState(){ - const raw=assistantText; + const raw=_stripXmlToolCalls(assistantText); if(reasoningText){ return {thinkingText:liveReasoningText, displayText:_streamDisplay(), inThinking:false}; } diff --git a/static/ui.js b/static/ui.js index 7affad7..4394df9 100644 --- a/static/ui.js +++ b/static/ui.js @@ -463,6 +463,17 @@ function getModelLabel(modelId){ return modelId.split('/').pop()||'Unknown'; } +function _stripXmlToolCallsDisplay(s){ + // Strip ... blocks emitted by DeepSeek and + // similar models in their raw response text. These are processed separately + // as tool calls; leaving them in the content causes them to render visibly + // in the settled chat bubble. (#702) + if(!s||s.toLowerCase().indexOf('')===-1) return s; + s=s.replace(/[\s\S]*?<\/function_calls>/gi,''); + s=s.replace(/[\s\S]*$/i,''); + return s.trim(); +} + function renderMd(raw){ let s=raw||''; // ── MEDIA: token stash (must run first, before any other processing) ─────── @@ -1459,7 +1470,7 @@ function renderMessages(){ if(m.attachments&&m.attachments.length){ filesHtml=`
${m.attachments.map(f=>`
${li('paperclip',12)} ${esc(f)}
`).join('')}
`; } - const bodyHtml = isUser ? esc(String(content)).replace(/\n/g,'
') : renderMd(String(content)); + const bodyHtml = isUser ? esc(String(content)).replace(/\n/g,'
') : renderMd(_stripXmlToolCallsDisplay(String(content))); const editBtn = isUser ? `` : ''; const retryBtn = isLastAssistant ? `` : ''; const copyBtn = ``; @@ -2135,6 +2146,20 @@ function renderFileTree(){ const box=$('fileTree');box.innerHTML=''; // Cache current dir entries S._dirCache[S.currentDir||'.']=S.entries; + // Show empty-state when no workspace is set or the directory is empty (#703) + const emptyEl=$('wsEmptyState'); + const hasWorkspace=!!(S.session&&S.session.workspace); + if(!hasWorkspace){ + if(emptyEl){emptyEl.textContent=t('workspace_empty_no_path');emptyEl.style.display='flex';} + box.style.display='none'; + return; + } + if(emptyEl) emptyEl.style.display='none'; + box.style.display=''; + if(!S.entries||!S.entries.length){ + if(emptyEl){emptyEl.textContent=t('workspace_empty_dir');emptyEl.style.display='flex';} + return; + } _renderTreeItems(box, S.entries, 0); } diff --git a/tests/test_sprint48.py b/tests/test_sprint48.py new file mode 100644 index 0000000..c015a08 --- /dev/null +++ b/tests/test_sprint48.py @@ -0,0 +1,209 @@ +"""Tests for sprint 48 UX bug fixes — v0.50.92. + +Covers: + - #702: XML tool-call syntax () stripped from assistant + message content before rendering (server-side + client-side). + - #703: Workspace file panel shows an empty-state message when no workspace + is configured or the directory is empty. + - #704: Notification settings description uses "app" instead of "tab". +""" + +import pathlib +import re + +REPO = pathlib.Path(__file__).parent.parent + + +def read(rel): + return (REPO / rel).read_text() + + +# ── Bug #702 — XML tool-call leak on DeepSeek ──────────────────────────────── + +class TestXmlToolCallStrip: + """_strip_xml_tool_calls() is defined in api/streaming.py and must remove + ... blocks from assistant content.""" + + def _load_fn(self): + """Import the helper from streaming.py without triggering full server + initialisation (which would fail in unit-test contexts).""" + import importlib, sys, types + + # Stub heavy transitive imports so we can import the module cleanly. + for mod in ('api.config', 'api.helpers', 'api.models', 'api.workspace'): + if mod not in sys.modules: + sys.modules[mod] = types.ModuleType(mod) + + # Provide minimal symbols that streaming.py needs at import time. + cfg = sys.modules.setdefault('api.config', types.ModuleType('api.config')) + for attr in ('STREAMS', 'STREAMS_LOCK', 'CANCEL_FLAGS', 'AGENT_INSTANCES', + 'LOCK', 'SESSIONS', 'SESSION_DIR', + '_get_session_agent_lock', '_set_thread_env', + '_clear_thread_env', 'resolve_model_provider'): + if not hasattr(cfg, attr): + setattr(cfg, attr, None) + + # Fall back to reading the source and exec-ing just the function. + src = read('api/streaming.py') + ns: dict = {} + # Extract the function definition with regex so we don't need to import + # the whole module (avoids all the heavy deps). + match = re.search( + r'(def _strip_xml_tool_calls\(.*?)\n(?=\ndef |\nclass )', + src, re.DOTALL + ) + assert match, "_strip_xml_tool_calls not found in api/streaming.py" + exec(compile('import re\n' + match.group(1), '', 'exec'), ns) + return ns['_strip_xml_tool_calls'] + + def test_complete_block_removed(self): + fn = self._load_fn() + text = "Hello foo world" + result = fn(text) + assert '' not in result + assert 'Hello' in result + assert 'world' in result + + def test_orphaned_opening_tag_removed(self): + fn = self._load_fn() + text = "Some answer text\n\ntool" + result = fn(text) + assert '' not in result + assert 'Some answer text' in result + + def test_no_tag_unchanged(self): + fn = self._load_fn() + text = "This is a normal response with no tool calls." + assert fn(text) == text + + def test_multiple_blocks_removed(self): + fn = self._load_fn() + text = ( + "Part one a " + "middle b end" + ) + result = fn(text) + assert '' not in result + assert 'Part one' in result + assert 'middle' in result + assert 'end' in result + + def test_function_defined_in_streaming_py(self): + src = read('api/streaming.py') + assert 'def _strip_xml_tool_calls(' in src, ( + "_strip_xml_tool_calls must be defined in api/streaming.py" + ) + + def test_strip_applied_to_assistant_messages(self): + """Verify the strip call is applied to assistant message content after + the agent run completes (server-side persistence fix).""" + src = read('api/streaming.py') + assert '_strip_xml_tool_calls' in src, ( + "_strip_xml_tool_calls must be referenced in api/streaming.py" + ) + # Confirm it is called on message content, not just defined + assert src.count('_strip_xml_tool_calls') >= 2, ( + "_strip_xml_tool_calls must be both defined and called" + ) + + def test_client_side_strip_in_messages_js(self): + src = read('static/messages.js') + assert '_stripXmlToolCalls' in src, ( + "Client-side _stripXmlToolCalls must exist in static/messages.js" + ) + assert 'function_calls' in src.lower(), ( + "Client-side strip must reference 'function_calls'" + ) + + def test_client_side_strip_in_ui_js(self): + src = read('static/ui.js') + assert '_stripXmlToolCallsDisplay' in src, ( + "_stripXmlToolCallsDisplay must exist in static/ui.js" + ) + + +# ── Bug #703 — Workspace file panel empty state ─────────────────────────────── + +class TestWorkspaceEmptyState: + + def test_i18n_no_path_string_present(self): + src = read('static/i18n.js') + assert 'workspace_empty_no_path' in src, ( + "i18n key workspace_empty_no_path must be defined in i18n.js" + ) + + def test_i18n_no_path_mentions_settings(self): + src = read('static/i18n.js') + # Extract the value of the key + m = re.search(r"workspace_empty_no_path:\s*'([^']+)'", src) + assert m, "workspace_empty_no_path value not found in i18n.js" + assert 'Settings' in m.group(1), ( + "workspace_empty_no_path should mention Settings" + ) + + def test_i18n_empty_dir_string_present(self): + src = read('static/i18n.js') + assert 'workspace_empty_dir' in src, ( + "i18n key workspace_empty_dir must be defined in i18n.js" + ) + + def test_empty_state_element_in_html(self): + src = read('static/index.html') + assert 'wsEmptyState' in src, ( + "id=\"wsEmptyState\" empty-state element must exist in index.html" + ) + + def test_render_file_tree_shows_empty_state(self): + src = read('static/ui.js') + assert 'wsEmptyState' in src, ( + "renderFileTree in ui.js must reference wsEmptyState" + ) + assert 'workspace_empty_no_path' in src, ( + "renderFileTree must use workspace_empty_no_path i18n key" + ) + assert 'workspace_empty_dir' in src, ( + "renderFileTree must use workspace_empty_dir i18n key" + ) + + +# ── Bug #704 — Notification description says "tab" ─────────────────────────── + +class TestNotificationDescriptionText: + + def test_english_uses_app_not_tab(self): + src = read('static/i18n.js') + # Find the English locale block (appears before other locales) + # The English block starts at line 1 (it's the first locale object). + # We look for the settings_desc_notifications in the English section. + # English block ends before the Spanish (es) block. + es_marker = "settings_desc_notifications: 'Muestra" + en_end = src.index(es_marker) if es_marker in src else len(src) + en_section = src[:en_end] + + m = re.search(r"settings_desc_notifications:\s*'([^']+)'", en_section) + assert m, "English settings_desc_notifications not found" + desc = m.group(1) + assert 'tab' not in desc.lower(), ( + f"English notification description must not say 'tab', got: {desc!r}" + ) + assert 'app' in desc.lower(), ( + f"English notification description must say 'app', got: {desc!r}" + ) + + def test_new_wording_exact(self): + src = read('static/i18n.js') + expected = 'while the app is in the background' + assert expected in src, ( + f"Exact phrase {expected!r} must appear in i18n.js" + ) + + def test_old_wording_removed_from_english(self): + src = read('static/i18n.js') + old_phrase = 'while the tab is in the background' + # The old phrase must not appear in the English locale section + es_marker = "settings_desc_notifications: 'Muestra" + en_end = src.index(es_marker) if es_marker in src else len(src) + en_section = src[:en_end] + assert old_phrase not in en_section, ( + "Old English notification description with 'tab' must be removed" + )