diff --git a/requirements-optional.txt b/requirements-optional.txt index ab21e81ee..efc49a833 100644 --- a/requirements-optional.txt +++ b/requirements-optional.txt @@ -33,4 +33,4 @@ PyMuPDF # magika (onnxruntime), already a core dep via fastembed. We avoid the # [all]/Azure/audio extras (cloud + heavy). Pinned to a release >30 days old per # the dependency-age discussion in issue #485. -markitdown[docx,pptx,xlsx,xls]==0.1.6 +markitdown[docx,pptx,xlsx,xls]==0.1.7 diff --git a/requirements.txt b/requirements.txt index 3c5114f53..1ed61edb2 100644 --- a/requirements.txt +++ b/requirements.txt @@ -3,9 +3,9 @@ uvicorn python-multipart python-dotenv httpx -httpcore>=1.0,<2.0 +httpcore>=1.0.9,<2.0 pydantic>=2.13.4 -pydantic-settings>=2.14.1 +pydantic-settings>=2.14.2 SQLAlchemy pypdf beautifulsoup4 diff --git a/routes/email_helpers.py b/routes/email_helpers.py index 257f5f921..c8639e1c7 100644 --- a/routes/email_helpers.py +++ b/routes/email_helpers.py @@ -247,7 +247,6 @@ import re as _re_reply _REPLY_OPEN_RE = _re_reply.compile(r"<<<\s*(?:REPLY|SUMMARY|OUTPUT)\s*>>+", _re_reply.I) _REPLY_CLOSE_RE = _re_reply.compile(r"<<<\s*END\s*>>+", _re_reply.I) _REPLY_ROLE_MARKER_RE = _re_reply.compile(r"?|?", _re_reply.I) -_SUMMARY_BULLET_RE = _re_reply.compile(r"^(?:[-*\u2022]\s+|\d+[.)]\s+)") def _extract_reply(text: str) -> str: @@ -278,125 +277,6 @@ def _extract_reply(text: str) -> str: return _strip_think(t).strip() -def _build_email_summary_messages(sender: str, subject: str, body_for_llm: str) -> list[dict[str, str]]: - return [ - { - "role": "system", - "content": ( - "You are an email summarizer. Format: 1-3 short bullet points " - "(use '- '). Cover: main point, action items, deadlines. If the " - "email has attachments (marked '--- ATTACHMENTS ---'), USE THEIR " - "CONTENTS - pull invoice totals, deadlines, key clauses, concrete " - "numbers/dates from PDFs/docs into the bullets. Be terse.\n\n" - "OUTPUT FORMAT: Put ONLY the bullet points between these exact " - "markers, each on its own line:\n" - "<<>>\n" - "- ...\n" - "<<>>\n" - "Any reasoning must come BEFORE <<>> (ideally inside " - "...). Only the text between the markers is kept." - ), - }, - { - "role": "user", - "content": ( - f"From: {sender}\nSubject: {subject}\n\n{body_for_llm[:12000]}" - "\n\n---\n\nSummarize the email. Output the bullets between " - "<<>> and <<>>." - ), - }, - ] - - -async def _generate_email_summary( - url: str, - model: str, - sender: str, - subject: str, - body_for_llm: str, - *, - headers: dict | None = None, - max_tokens: int = 8192, - timeout: int = 180, -) -> str: - """Generate an interactive email summary through the shared LLM adapter.""" - from src.llm_core import llm_call_async - - raw = await llm_call_async( - url=url, - model=model, - messages=_build_email_summary_messages(sender, subject, body_for_llm), - temperature=0.3, - max_tokens=max_tokens, - headers=headers, - timeout=timeout, - workload="foreground", - ) - return _normalize_email_summary(raw) - - -async def _generate_scheduled_email_summary( - url: str, - model: str, - sender: str, - subject: str, - body_for_llm: str, - *, - headers: dict | None = None, - owner: str | None = None, - max_tokens: int = 8192, - timeout: int = 180, -) -> str: - """Generate a scheduled summary through the background task candidate chain.""" - from src.task_endpoint import task_llm_call_async - - raw = await task_llm_call_async( - messages=_build_email_summary_messages(sender, subject, body_for_llm), - fallback_url=url, - fallback_model=model, - fallback_headers=headers, - owner=owner, - temperature=0.3, - max_tokens=max_tokens, - timeout=timeout, - ) - return _normalize_email_summary(raw) - - -def _normalize_email_summary(raw) -> str: - """Extract a stable cache/UI summary from provider output.""" - raw_text = raw or "" - if _REPLY_OPEN_RE.search(raw_text): - summary = _extract_reply(raw_text) - if summary: - return summary - - cleaned = _strip_think(raw_text).strip() - bullets = [ - line.strip() - for line in cleaned.splitlines() - if _SUMMARY_BULLET_RE.match(line.strip()) - ] - if bullets: - return "\n".join(bullets) - return cleaned.strip() - - -EMAIL_SUMMARY_ERROR_CODE = "email_summary_unavailable" -EMAIL_SUMMARY_ERROR_MESSAGE = "Failed to summarize" - - -def _email_summary_failure_log_detail(exc: BaseException) -> str: - """Return useful provider-failure metadata without echoing exception text.""" - detail = f"type={type(exc).__name__}" - status = getattr(exc, "status_code", None) - if status is None: - status = getattr(getattr(exc, "response", None), "status_code", None) - if isinstance(status, int): - detail += f" status={status}" - return detail - - def _apply_email_style_mechanics(text: str) -> str: """Enforce deterministic writing-style mechanics that models often miss.""" if not text: diff --git a/routes/email_pollers.py b/routes/email_pollers.py index a2507989d..5d96bd0f9 100644 --- a/routes/email_pollers.py +++ b/routes/email_pollers.py @@ -40,7 +40,6 @@ from routes.email_helpers import ( _pre_retrieve_context, _attach_compose_uploads, _cleanup_compose_uploads, _q, SCHEDULED_DB, _EMAIL_REPLY_SYS_PROMPT_BASE, _email_cache_owner_clause, - _generate_scheduled_email_summary, _email_summary_failure_log_detail, ) logger = logging.getLogger(__name__) @@ -654,7 +653,6 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None no_msgid = 0 examined = 0 _summaries_created = 0 - _summary_failed = 0 _events_created = 0 _replies_drafted = 0 _reply_failed = 0 @@ -787,17 +785,16 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None if need_sum: try: - summary = await _generate_scheduled_email_summary( - url=url, - model=model, - sender=sender, - subject=subject, - body_for_llm=body_for_llm, - headers=req_headers, + summary = await task_llm_call_async( + messages=[ + {"role": "system", "content": "You are an email summarizer. Format: 1-3 short bullet points (use '- '). Cover: main point, action items, deadlines. If the email has attachments (marked '--- ATTACHMENTS ---'), USE THEIR CONTENTS — pull out invoice totals, deadlines, key clauses, any concrete numbers/dates in PDFs/docs, and reflect them in the bullets. Be terse.\n\nOUTPUT FORMAT: Put ONLY the bullet points between these exact markers, each on its own line:\n<<>>\n- ...\n<<>>\nAny reasoning or planning must come BEFORE <<>> (ideally inside ...). Only the text between the markers is kept."}, + {"role": "user", "content": f"From: {sender}\nSubject: {subject}\n\n{body_for_llm[:12000]}\n\n---\n\nSummarize the email. Output the bullets between <<>> and <<>>."}, + ], + fallback_url=url, fallback_model=model, fallback_headers=headers, owner=account_owner or None, - max_tokens=16384, - timeout=240, + temperature=0.3, max_tokens=16384, timeout=240, ) + summary = _extract_reply((summary or "").strip()) if summary: _c = _sql3.connect(SCHEDULED_DB) _c.execute(""" @@ -811,19 +808,10 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None _summaries_created += 1 _uid_text = uid.decode() if isinstance(uid, bytes) else str(uid) _detail_lines.append(f"summary · {_folder}#{_uid_text} · {subject or '(no subject)'} — {sender or '(unknown sender)'}") - else: - _summary_failed += 1 - _uid_text = uid.decode() if isinstance(uid, bytes) else str(uid) - _detail_lines.append(f"summary empty · {_folder}#{_uid_text} · {subject or '(no subject)'} — {sender or '(unknown sender)'}") except Exception as e: - _summary_failed += 1 _uid_text = uid.decode() if isinstance(uid, bytes) else str(uid) _detail_lines.append(f"summary failed · {_folder}#{_uid_text} · {subject or '(no subject)'} — {sender or '(unknown sender)'}") - logger.warning( - "Auto-summary uid=%s failed %s", - _uid_text, - _email_summary_failure_log_detail(e), - ) + logger.warning(f"Auto-summary {uid} failed: {e}") if need_reply: await _emit_progress(progress_cb, f"Drafting reply {processed + 1}/{_max_process} · checked {examined}/{len(uid_list)}") @@ -1332,8 +1320,6 @@ async def _auto_summarize_pass_single(days_back: int = 1, account_id: str | None parts.append(f"processed {processed} new") if auto_sum: parts.append(f"summarized {_summaries_created}") - if _summary_failed: - parts.append(f"{_summary_failed} summary failed") if auto_reply_draft: parts.append(f"drafted {_replies_drafted} repl" + ("y" if _replies_drafted == 1 else "ies")) if _reply_failed: diff --git a/routes/email_routes.py b/routes/email_routes.py index 76a744ce1..3c8e407bd 100644 --- a/routes/email_routes.py +++ b/routes/email_routes.py @@ -57,8 +57,7 @@ from routes.email_helpers import ( _extract_attachment_to_disk, _extract_html, _extract_text, _fetch_sender_thread_context, _pre_retrieve_context, _EMAIL_REPLY_SYS_PROMPT_BASE, _POOL_HOOKS, - _friendly_email_auth_error, _email_summary_failure_log_detail, - _generate_email_summary, EMAIL_SUMMARY_ERROR_CODE, EMAIL_SUMMARY_ERROR_MESSAGE, + _friendly_email_auth_error, SendEmailRequest, ExtractStyleRequest, ATTACHMENTS_DIR, COMPOSE_UPLOADS_DIR, SCHEDULED_DB, attachment_extract_dir, _email_cache_owner_clause, email_translation_body_hash, @@ -4767,6 +4766,8 @@ def setup_email_routes(): """Generate a quick AI summary of an email body.""" try: from src.endpoint_resolver import resolve_endpoint + from src.llm_core import _uses_max_completion_tokens, _restricts_temperature + import requests as _req body = data.get("body", "") subject = data.get("subject", "") @@ -4777,11 +4778,7 @@ def setup_email_routes(): if account_id: _assert_owns_account(account_id, owner) if not body: - return { - "success": False, - "error": "No body provided", - "error_code": "email_summary_missing_body", - } + return {"success": False, "error": "No body provided"} # If we know which UID this is, fetch the raw message and pull # attachment text so the summary can reference invoice totals, @@ -4810,43 +4807,53 @@ def setup_email_routes(): if not url: url, model, headers = resolve_endpoint("default", owner=owner) if not url or not model: - return { - "success": False, - "error": "No model configured for email summaries", - "error_code": "email_summary_not_configured", - } + return {"success": False, "error": "No LLM endpoint configured"} req_headers = {"Content-Type": "application/json"} if headers: req_headers.update(headers) - try: - content = await _generate_email_summary( - url=url, - model=model, - sender=sender, - subject=subject, - body_for_llm=body_for_llm, - headers=req_headers, - max_tokens=8192, - timeout=180, - ) - except Exception as e: - logger.warning( - "Email summary LLM call failed %s", - _email_summary_failure_log_detail(e), - ) - return { - "success": False, - "error": EMAIL_SUMMARY_ERROR_MESSAGE, - "error_code": EMAIL_SUMMARY_ERROR_CODE, - } + tok_key = "max_completion_tokens" if _uses_max_completion_tokens(model) else "max_tokens" + payload = { + "model": model, + "messages": [ + {"role": "system", "content": "You are an email summarizer. Format: 1-3 short bullet points (use '- '). Cover: main point, action items, deadlines. If the email has attachments (marked '--- ATTACHMENTS ---'), USE THEIR CONTENTS — pull invoice totals, deadlines, key clauses, concrete numbers/dates from PDFs/docs into the bullets. Be terse.\n\nOUTPUT FORMAT: Put ONLY the bullet points between these exact markers, each on its own line:\n<<>>\n- ...\n<<>>\nAny reasoning must come BEFORE <<>> (ideally inside ...). Only the text between the markers is kept."}, + {"role": "user", "content": f"From: {sender}\nSubject: {subject}\n\n{body_for_llm[:12000]}\n\n---\n\nSummarize the email. Output the bullets between <<>> and <<>>."}, + ], + tok_key: 8192, + "temperature": 0.3, + "stream": False, + } + # Reasoning models (o1/o3/o4/gpt-5) reject an explicit temperature. + if _restricts_temperature(model): + payload.pop("temperature", None) + resp = await asyncio.to_thread( + _req.post, url, json=payload, headers=req_headers, timeout=180 + ) + if not resp.ok: + return {"success": False, "error": f"LLM HTTP {resp.status_code}"} + rdata = resp.json() + msg = (rdata.get("choices") or [{}])[0].get("message", {}) + content = (msg.get("content") or "").strip() + content = _extract_reply(content) if not content: - return { - "success": False, - "error": "The model returned an empty summary", - "error_code": "email_summary_empty", - } + # Model put everything in reasoning_content — extract bullet points + rc = (msg.get("reasoning_content") or "").strip() + # Find bullet-point style output (lines starting with -, •, *, or numbered) + bullet_lines = [] + for line in rc.split("\n"): + stripped = line.strip() + if re.match(r"^[-•*]\s+|^\d+[.)]\s+", stripped): + bullet_lines.append(stripped) + if bullet_lines: + content = "\n".join(bullet_lines) + else: + # Last resort: take the last paragraph + paragraphs = [p.strip() for p in rc.split("\n\n") if p.strip()] + content = paragraphs[-1] if paragraphs else rc[:500] + + if not content: + return {"success": False, "error": "Empty response from model"} # Cache the summary if we have a message_id mid = data.get("message_id", "") @@ -4869,15 +4876,8 @@ def setup_email_routes(): return {"success": True, "summary": content, "model_used": model} except Exception as e: - logger.error( - "Email summary route failed %s", - _email_summary_failure_log_detail(e), - ) - return { - "success": False, - "error": EMAIL_SUMMARY_ERROR_MESSAGE, - "error_code": EMAIL_SUMMARY_ERROR_CODE, - } + logger.error(f"Failed to summarize: {e}") + return {"success": False, "error": "Mail operation failed"} @router.post("/translate") async def translate_email(data: dict, owner: str = Depends(require_owner)): diff --git a/src/tool_parsing.py b/src/tool_parsing.py index 98dc1b5f6..2885cc00f 100644 --- a/src/tool_parsing.py +++ b/src/tool_parsing.py @@ -187,12 +187,8 @@ _FUNCTION_MODEL_NAME_RE = re.compile( _FUNCTION_MODEL_PARAMS_OPEN_RE = re.compile(r"\s*", re.IGNORECASE) _FUNCTION_MODEL_PARAMS_CLOSE_RE = re.compile(r"", re.IGNORECASE) _QWEN_ROLE_MARKER_RE = re.compile(r"?|?", re.IGNORECASE) -# At least one pipe is required around `end`. Both pipes used to be optional -# (`\|?end\|?`), which also matched a bare `end` on its own line and deleted it -# from ordinary prose and from Ruby/Lua/shell snippets that close blocks with -# one; see #5547. `|end`, `end|`, `|end|` and `/|end|` still strip as before. _QWEN_BARE_MARKER_RE = re.compile( - r"(?:^|[\t\r\n ])(?:/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|" + r"(?:^|[\t\r\n ])(?:\|?end\|?|/?\|end\|)(?=[\t\r\n ]|$)|" r"(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)", re.IGNORECASE, ) diff --git a/static/app.js b/static/app.js index 2f1e8d4bf..97f0ae77e 100644 --- a/static/app.js +++ b/static/app.js @@ -10,14 +10,14 @@ import modelsModule from './js/models.js?v=20260715startupcalm2'; import ragModule from './js/rag.js'; import presetsModule from './js/presets.js'; import searchModule from './js/search.js'; -import chatModule from './js/chat.js?v=20260801fix1'; +import chatModule from './js/chat.js?v=20260722ctxheader4'; import compareModule from './js/compare/index.js?v=20260723compareicon2'; import documentModule from './js/document.js?v=20260722emailfastindex1'; import searchChatModule from './js/search-chat.js'; import { makeWindowDraggable } from './js/windowDrag.js'; import markdownModule from './js/markdown.js'; import chatRenderer from './js/chatRenderer.js?v=20260722emailfastindex1'; -import sessionModule from './js/sessions.js'; +import sessionModule from './js/sessions.js?v=20260722ctxheader4'; import memoryModule from './js/memory.js?v=20260722memoryloading1'; import voiceRecorderModule from './js/voiceRecorder.js'; import censorModule from './js/censor.js'; @@ -1689,20 +1689,12 @@ function initializeEventListeners() { const newMemoryInput = el('new-memory-input'); if (newMemoryInput) { - // keydown, not the deprecated keypress: keypress is not guaranteed to - // fire for Enter everywhere, which left the Add Memory form with no - // working submit path (#5828). - newMemoryInput.addEventListener('keydown', (e) => { - if (e.key === 'Enter' && !e.isComposing) { - e.preventDefault(); + newMemoryInput.addEventListener('keypress', (e) => { + if (e.key === 'Enter') { memoryModule.addNewMemory(); } }); } - const newMemoryAddBtn = el('new-memory-add-btn'); - if (newMemoryAddBtn) { - newMemoryAddBtn.addEventListener('click', () => memoryModule.addNewMemory()); - } // Voice recording is handled by the dual-purpose send/mic button (see below) @@ -3916,10 +3908,85 @@ function startOdysseusApp() { const messageInput = el('message'); const modelPickerWrap = document.getElementById('model-picker-wrap'); - // ArrowUp/ArrowDown prompt recall on #message lives in - // static/js/composerArrowUpRecall.js (wired from chat.js). Do not re-add a - // copy here: two capture-phase listeners on the same textarea meant the one - // without the draft guard won and ate unsent multi-line prompts (#5862). + function _readComposerPromptHistory() { + const chatBox = document.getElementById('chat-history'); + if (!chatBox) return []; + return Array.from(chatBox.querySelectorAll('.msg-user')) + .reverse() + .map(msg => { + const body = msg.querySelector('.body'); + return msg.dataset?.raw || (body ? body.textContent : '') || ''; + }) + .filter(Boolean); + } + + if (messageInput && !messageInput._odysseusPromptRecallCapture) { + messageInput._odysseusPromptRecallCapture = true; + let recallHistory = []; + let recallIndex = -1; + let lastRecalled = ''; + const norm = (v) => String(v || '').replace(/\r\n/g, '\n').trimEnd(); + messageInput.addEventListener('input', () => { + if (norm(messageInput.value) === norm(lastRecalled)) return; + recallHistory = []; + recallIndex = -1; + lastRecalled = ''; + try { delete messageInput.dataset.odysseusRecallIndex; } catch {} + }, true); + messageInput.addEventListener('keydown', (e) => { + if (e.key !== 'ArrowUp' && e.key !== 'ArrowDown') return; + if (e.shiftKey || e.altKey || e.ctrlKey || e.metaKey || e.isComposing) return; + if (window._ghostAutocomplete?.isActive?.()) return; + const fresh = _readComposerPromptHistory(); + const history = fresh.length ? fresh : recallHistory; + if (!history.length) return; + const current = norm(messageInput.value); + let currentIndex = current ? history.findIndex(item => norm(item) === current) : -1; + if (current && currentIndex < 0 && current === norm(lastRecalled)) currentIndex = recallIndex; + if (current && currentIndex < 0) { + const markedIndex = Number(messageInput.dataset.odysseusRecallIndex); + if (Number.isInteger(markedIndex) && markedIndex >= 0 && markedIndex < history.length) { + currentIndex = markedIndex; + } + } + e.preventDefault(); + e.stopPropagation(); + e.stopImmediatePropagation(); + if (e.key === 'ArrowDown') { + if (currentIndex < 0) return; + const nextIndex = currentIndex - 1; + if (nextIndex < 0) { + recallHistory = history; + recallIndex = -1; + lastRecalled = ''; + try { delete messageInput.dataset.odysseusRecallIndex; } catch {} + messageInput.value = ''; + try { messageInput.selectionStart = messageInput.selectionEnd = 0; } catch {} + try { uiModule.autoResize(messageInput); } catch {} + return; + } + const recalled = history[nextIndex]; + recallHistory = history; + recallIndex = nextIndex; + lastRecalled = recalled; + try { messageInput.dataset.odysseusRecallIndex = String(nextIndex); } catch {} + messageInput.value = recalled; + try { messageInput.selectionStart = messageInput.selectionEnd = recalled.length; } catch {} + try { uiModule.autoResize(messageInput); } catch {} + return; + } + const nextIndex = currentIndex >= 0 ? Math.min(currentIndex + 1, history.length - 1) : 0; + const recalled = history[nextIndex]; + if (!recalled) return; + recallHistory = history; + recallIndex = nextIndex; + lastRecalled = recalled; + try { messageInput.dataset.odysseusRecallIndex = String(nextIndex); } catch {} + messageInput.value = recalled; + try { messageInput.selectionStart = messageInput.selectionEnd = recalled.length; } catch {} + try { uiModule.autoResize(messageInput); } catch {} + }, true); + } const _sendIcon = ''; const _micIcon = ''; diff --git a/static/index.html b/static/index.html index fea4e20ac..8257660fe 100644 --- a/static/index.html +++ b/static/index.html @@ -250,9 +250,9 @@ - + - + @@ -365,7 +365,6 @@ Add a memory — e.g. 'I prefer concise replies' -
@@ -1006,7 +1005,7 @@ var tips = mobile ? phone : desktop; var el = document.getElementById('welcome-tip'); if (el) { - el.textContent = tips[Math.floor(Math.random() * tips.length)]; + el.textContent = 'Pick a model if you want, or just type.'; } fetch('/api/version').then(function(r){return r.json()}).then(function(d){ if (d.version) window._appVersion = d.version; @@ -2505,7 +2504,7 @@ - + @@ -2523,7 +2522,7 @@ - + diff --git a/static/js/chat.js b/static/js/chat.js index 3c8bbe850..ea2d8c1bb 100644 --- a/static/js/chat.js +++ b/static/js/chat.js @@ -349,9 +349,6 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr async function _adoptOpenedSessionBeforeAutoCreate() { if (!sessionModule || !sessionModule.getCurrentSessionId || sessionModule.getCurrentSessionId()) return true; - // Don't adopt a stale session when the user explicitly started a New Chat - // (pending state set) — the send path must materialize the pending session. - if (sessionModule.hasPendingChat && sessionModule.hasPendingChat()) return false; const activeRowId = document.querySelector('.list-item.active-session[data-session-id], .session-item.active[data-session-id]')?.dataset?.sessionId || ''; const hashId = _hashSessionCandidate(); const lastSelectedId = String(window.__odysseusLastSelectedSessionId || '').trim(); @@ -1406,8 +1403,6 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr currentAccumulated = ''; currentHolder = null; - let abortCtrl = null; - let streamingTTS = false; try { // Re-enable auto-scroll when user sends a message uiModule.setAutoScroll(true); @@ -1721,7 +1716,7 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr } - abortCtrl = new AbortController(); + const abortCtrl = new AbortController(); abortCtrl._reason = ''; currentAbort = abortCtrl; @@ -1902,7 +1897,7 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr let isThinking = false; let thinkingStartTime = null; // Streaming TTS: synthesize sentence-by-sentence during streaming - streamingTTS = !!(window.aiTTSManager && window.aiTTSManager.autoPlay && window.aiTTSManager.available); + const streamingTTS = !!(window.aiTTSManager && window.aiTTSManager.autoPlay && window.aiTTSManager.available); if (streamingTTS) window.aiTTSManager.streamingStart(); // Multi-bubble agent tracking let roundHolder = holder; // Current AI text bubble (changes per round) @@ -4792,8 +4787,7 @@ import { wireArrowUpRecall, getUserMessagesFromChatHistory } from './composerArr if (msgIndex < 0) return; const bodyEl = userMsgElement.querySelector('.body'); - let currentText = (userMsgElement.dataset.raw || (bodyEl ? bodyEl.textContent : '') || '').trim(); - currentText = currentText.replace(/\s*\[\d+ attachment\(s\)\]$/, ''); + const currentText = bodyEl ? bodyEl.textContent.trim().replace(/\s*\[\d+ attachment\(s\)\]$/, '') : ''; // Replace body with an editable textarea const editor = document.createElement('textarea'); diff --git a/static/js/chatRenderer.js b/static/js/chatRenderer.js index 1d6e2e4a9..10709679d 100644 --- a/static/js/chatRenderer.js +++ b/static/js/chatRenderer.js @@ -478,10 +478,7 @@ const DSML_STRAY_RE = /<\s*\/?\s*[||]+\s*DSML\s*[||]+[^>]*>/gi; const DSML_INVOKE_RE = /<\s*[||]+\s*DSML\s*[||]+\s*invoke\b[^>]*>[\s\S]*?(?:<\s*\/\s*[||]+\s*DSML\s*[||]+\s*invoke\s*>|$)/gi; const RAW_OPENAI_TOOL_JSON_RE = /(?:\[\s*)?\{\s*"function"\s*:\s*\{[\s\S]*?\}\s*,\s*"id"\s*:\s*"[^"]*"\s*,\s*"type"\s*:\s*"function"\s*\}\s*\]?/gi; const QWEN_ROLE_MARKER_RE = /<\/?\|(?:assistant|assistan|user|system|tool)\|>?|<\/\|end\|>?/gi; -// Keep in sync with _QWEN_BARE_MARKER_RE in src/tool_parsing.py. At least one -// pipe is required around `end`: with both optional (`\|?end\|?`) this also ate -// a bare `end` on its own line, breaking Ruby/Lua/shell snippets (#5547). -const QWEN_BARE_MARKER_RE = /(?:^|[\t\r\n ])(?:\/?\|end\||\|end|end\|)(?=[\t\r\n ]|$)|(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)/gi; +const QWEN_BARE_MARKER_RE = /(?:^|[\t\r\n ])(?:\|?end\|?|\/?\|end\|)(?=[\t\r\n ]|$)|(?:^|[\t\r\n ])assistan(?:t)?(?=[\t\r\n ]|$)/gi; // Self-narration about tool results (model echoing stdout/exit_code) const TOOL_NARRATION_RE = /(?:The (?:result|output) shows?:?\s*)?-?\s*(?:stdout|stderr|exit_code):\s*.+/gi; diff --git a/static/js/composerArrowUpRecall.js b/static/js/composerArrowUpRecall.js index 83141bfe9..e0b20d6b4 100644 --- a/static/js/composerArrowUpRecall.js +++ b/static/js/composerArrowUpRecall.js @@ -143,9 +143,9 @@ export function wireArrowUpRecall(composer, getUserMessages, options = {}) { return; } - // ArrowUp walks older prompts. An unmatched draft already returned above, - // so reaching here means the composer is empty or holds a recalled prompt - // — the caret-navigation case is never hijacked. + // ArrowUp owns prompt history in the chat composer. If the current text + // is not already a recalled prompt, start from newest instead of letting + // the browser move the caret inside the textarea. const nextIndex = currentIndex >= 0 ? Math.min(currentIndex + 1, history.length - 1) : 0; const recalled = history[nextIndex]; if (!recalled) { diff --git a/static/js/emailLibrary.js b/static/js/emailLibrary.js index 32b906ddc..6a0d3e294 100644 --- a/static/js/emailLibrary.js +++ b/static/js/emailLibrary.js @@ -13,7 +13,7 @@ import { makeWindowDraggable } from './windowDrag.js'; import { _esc, _escLinkify, _extractName, _parseTurnMeta, _formatBubbleDate, _formatRecipients, _senderColor, _initials, - _sanitizeHtml, _renderEmailSummaryError, + _sanitizeHtml, _TALON_WROTE, _TALON_FROM, _TALON_SENT, _TALON_SUBJ, _TALON_TO, _TALON_ORIG_RE, _SIG_BLOAT_MIN_CHARS, } from './emailLibrary/utils.js'; @@ -7259,11 +7259,12 @@ async function _generateSummary(reader, data, btn) { if (label) label.textContent = 'Summary'; } } else { - _renderEmailSummaryError(content, result); + content.innerHTML = `${_esc(result.error || 'Failed to summarize')}`; + panel.remove(); } } catch (e) { sp.destroy(); - _renderEmailSummaryError(content, null); + panel.remove(); if (uiModule) uiModule.showError?.('Failed to summarize'); } finally { if (btn) btn.disabled = false; diff --git a/static/js/emailLibrary/utils.js b/static/js/emailLibrary/utils.js index f634c9949..82a5c86ec 100644 --- a/static/js/emailLibrary/utils.js +++ b/static/js/emailLibrary/utils.js @@ -30,25 +30,6 @@ export function _esc(text) { return div.innerHTML; } -const _EMAIL_SUMMARY_ERROR_MESSAGES = Object.freeze({ - email_summary_missing_body: 'No email body to summarize', - email_summary_not_configured: 'No model configured for email summaries', - email_summary_empty: 'The model returned an empty summary', - email_summary_unavailable: 'Failed to summarize', -}); - -export function _emailSummaryErrorMessage(result) { - const code = String(result?.error_code || ''); - return _EMAIL_SUMMARY_ERROR_MESSAGES[code] || 'Failed to summarize'; -} - -export function _renderEmailSummaryError(container, result) { - const message = container.ownerDocument.createElement('span'); - message.style.color = 'var(--red)'; - message.textContent = _emailSummaryErrorMessage(result); - container.replaceChildren(message); -} - function _attrEsc(text) { return String(text ?? '') .replace(/"/g, '"') diff --git a/static/js/sessions.js b/static/js/sessions.js index edf83c8a4..cf59d478c 100644 --- a/static/js/sessions.js +++ b/static/js/sessions.js @@ -1847,10 +1847,6 @@ export async function selectSession(id, { keepSidebar = false, showLoading = tru const _isTransientChat = !!_meta && (_meta.folder === 'Assistant' || _meta.folder === 'Tasks'); if (!_isTransientChat) { Storage.set('lastSessionId', id); - // Update URL hash without triggering hashchange handler - if (window.location.hash !== '#' + id) { - history.replaceState(null, '', '#' + id); - } } // Restore character preset for persistent chats try { @@ -2317,7 +2313,6 @@ export async function materializePendingSession() { currentSessionId = payload.id; if (!isIncognito) { Storage.set('lastSessionId', payload.id); - history.replaceState(null, '', '#' + payload.id); } // Reload the sidebar in the background. Awaiting this used to block the first diff --git a/static/js/skills.js b/static/js/skills.js index b45403570..84974d446 100644 --- a/static/js/skills.js +++ b/static/js/skills.js @@ -83,9 +83,11 @@ export async function loadSkills(cascade = false) { // Play the domino-in entrance on this load (set when the tab is opened, // not for the silent re-loads after an edit/delete). if (cascade) _cascadeNext = true; - // Always re-fetch when the tab is explicitly opened — the cascade - // animation is handled inside renderSkillsList() via _cascadeNext. - // Skipping the fetch here caused stale data on panel close/reopen (#5870). + if (cascade && loaded && !_loadPromise && _playSkillsCascade()) { + _cascadeNext = false; + updateCount(); + return; + } if (_loadPromise) return _loadPromise; _loadPromise = (async () => { try { diff --git a/tests/test_composer_arrow_up_recall_js.py b/tests/test_composer_arrow_up_recall_js.py index 022fcbc02..eadc3bc94 100644 --- a/tests/test_composer_arrow_up_recall_js.py +++ b/tests/test_composer_arrow_up_recall_js.py @@ -306,24 +306,3 @@ def test_integration_recalls_from_chat_history_dom(): ) assert proc.returncode == 0, proc.stderr assert json.loads(proc.stdout.strip()) == {"value": "stored prompt", "prevented": True} - - -def test_prompt_recall_is_not_duplicated_in_app_js(): - """Only composerArrowUpRecall.js may own ArrowUp on #message (issue #5862). - - static/app.js once carried a near-verbatim copy of this recall logic, wired - as a second capture-phase listener on the same textarea. That copy lacked - the draft guard here, and because it called stopImmediatePropagation it won - regardless of registration order — so a typed multi-line prompt was replaced - by the last sent one instead of the caret moving up a line. - """ - app_js = (_REPO / "static" / "app.js").read_text(encoding="utf-8") - for marker in ( - "_odysseusPromptRecallCapture", - "_readComposerPromptHistory", - "odysseusRecallIndex", - ): - assert marker not in app_js, ( - f"static/app.js reintroduces prompt recall ({marker!r}); " - "it belongs to static/js/composerArrowUpRecall.js alone" - ) diff --git a/tests/test_email_summary_error_ui_js.py b/tests/test_email_summary_error_ui_js.py deleted file mode 100644 index 1afc3bec9..000000000 --- a/tests/test_email_summary_error_ui_js.py +++ /dev/null @@ -1,52 +0,0 @@ -import json -import shutil -import subprocess -from pathlib import Path - -import pytest - - -_REPO = Path(__file__).resolve().parent.parent -_UTILS = (_REPO / "static" / "js" / "emailLibrary" / "utils.js").as_posix() -_HAS_NODE = shutil.which("node") is not None - -pytestmark = pytest.mark.skipif(not _HAS_NODE, reason="node binary not on PATH") - - -def test_email_summary_renderer_ignores_untrusted_provider_error_text(): - secret = ( - "endpoint=https://private.example.internal/v1 provider=ollama " - "model=private-model response_body=private-response " - "Authorization: Bearer token-secret-value" - ) - script = f""" - import {{ _renderEmailSummaryError }} from '{_UTILS}'; - const host = {{ - ownerDocument: {{ - createElement() {{ return {{ style: {{}}, textContent: '' }}; }}, - }}, - replaceChildren(node) {{ this.child = node; }}, - }}; - _renderEmailSummaryError(host, {{ - error_code: 'email_summary_unavailable', - error: {json.dumps(secret)}, - }}); - console.log(JSON.stringify({{ - text: host.child.textContent, - color: host.child.style.color, - }})); - """ - - proc = subprocess.run( - ["node", "--input-type=module"], - input=script, - capture_output=True, - text=True, - cwd=str(_REPO), - timeout=30, - ) - - assert proc.returncode == 0, proc.stderr - rendered = json.loads(proc.stdout) - assert rendered == {"text": "Failed to summarize", "color": "var(--red)"} - assert secret not in proc.stdout diff --git a/tests/test_email_summary_llm.py b/tests/test_email_summary_llm.py deleted file mode 100644 index b0ab7b3be..000000000 --- a/tests/test_email_summary_llm.py +++ /dev/null @@ -1,406 +0,0 @@ -import asyncio -import json -import logging -import os -import sqlite3 -import sys -import tempfile -from pathlib import Path - -import pytest - - -_TMP_DATA = Path(tempfile.mkdtemp(prefix="odysseus-email-summary-")) -os.environ.setdefault("DATA_DIR", str(_TMP_DATA)) -os.environ.setdefault("DATABASE_URL", f"sqlite:///{_TMP_DATA / 'app.db'}") - -PROJECT_ROOT = Path(__file__).resolve().parent.parent -if str(PROJECT_ROOT) not in sys.path: - sys.path.insert(0, str(PROJECT_ROOT)) - - -def _route_endpoint(router, path: str, method: str): - method = method.upper() - for route in router.routes: - if route.path == path and method in getattr(route, "methods", set()): - return route.endpoint - raise AssertionError(f"route not found: {method} {path}") - - -@pytest.mark.asyncio -async def test_generate_email_summary_uses_shared_llm_adapter(monkeypatch): - import routes.email_helpers as email_helpers - import src.llm_core as llm_core - - calls = {} - - async def fake_llm_call_async(url, model, messages, **kwargs): - calls["url"] = url - calls["model"] = model - calls["messages"] = messages - calls["kwargs"] = kwargs - return "thinking before marker\n<<>>\n- Pay the invoice by Friday.\n<<>>" - - monkeypatch.setattr(llm_core, "llm_call_async", fake_llm_call_async) - - summary = await email_helpers._generate_email_summary( - url="https://chatgpt.com/backend-api/codex/responses", - model="gpt-5.5", - sender="Billing ", - subject="Invoice due", - body_for_llm="Please pay invoice 123 by Friday.", - headers={"Authorization": "Bearer test"}, - max_tokens=1234, - timeout=45, - ) - - assert summary == "- Pay the invoice by Friday." - assert calls["url"] == "https://chatgpt.com/backend-api/codex/responses" - assert calls["model"] == "gpt-5.5" - assert calls["kwargs"]["headers"] == {"Authorization": "Bearer test"} - assert calls["kwargs"]["temperature"] == 0.3 - assert calls["kwargs"]["max_tokens"] == 1234 - assert calls["kwargs"]["timeout"] == 45 - assert calls["kwargs"]["workload"] == "foreground" - assert calls["messages"][0]["role"] == "system" - assert calls["messages"][1]["role"] == "user" - - -@pytest.mark.asyncio -async def test_scheduled_email_summary_uses_background_fallback_chain(monkeypatch): - import routes.email_helpers as email_helpers - import src.llm_core as llm_core - import src.task_endpoint as task_endpoint - - candidates = [ - ("http://primary.invalid/v1", "primary-model", {"X-Candidate": "primary"}), - ("http://fallback.invalid/v1", "fallback-model", {"X-Candidate": "fallback"}), - ] - resolve_calls = [] - wait_calls = [] - llm_calls = [] - - def fake_resolve_task_candidates(**kwargs): - resolve_calls.append(kwargs) - return candidates - - async def fake_wait_for_interactive_quiet(label): - wait_calls.append(label) - return False - - async def fake_llm_call_async(url, model, messages, **kwargs): - llm_calls.append((url, model, messages, kwargs)) - if model == "primary-model": - raise RuntimeError("primary unavailable") - return "<<>>\n- Used the fallback model.\n<<>>" - - monkeypatch.setattr(task_endpoint, "resolve_task_candidates", fake_resolve_task_candidates) - monkeypatch.setattr(task_endpoint, "wait_for_interactive_quiet", fake_wait_for_interactive_quiet) - monkeypatch.setattr(llm_core, "llm_call_async", fake_llm_call_async) - - summary = await email_helpers._generate_scheduled_email_summary( - url="http://caller-fallback.invalid/v1", - model="caller-fallback-model", - sender="Sender ", - subject="Scheduled subject", - body_for_llm="Please summarize this scheduled email.", - headers={"Authorization": "Bearer test"}, - owner="alice", - max_tokens=321, - timeout=54, - ) - - assert summary == "- Used the fallback model." - assert resolve_calls == [{ - "fallback_url": "http://caller-fallback.invalid/v1", - "fallback_model": "caller-fallback-model", - "fallback_headers": {"Authorization": "Bearer test"}, - "owner": "alice", - }] - assert wait_calls == ["background task LLM"] - assert [call[1] for call in llm_calls] == ["primary-model", "fallback-model"] - assert all(call[3]["workload"] == "background" for call in llm_calls) - assert all(call[3]["max_tokens"] == 321 for call in llm_calls) - assert all(call[3]["timeout"] == 54 for call in llm_calls) - - -@pytest.mark.asyncio -async def test_scheduled_local_summary_is_preempted_by_foreground_call(monkeypatch): - import routes.email_helpers as email_helpers - import src.llm_core as llm_core - import src.task_endpoint as task_endpoint - - local_url = "http://127.0.0.1:11434/v1/chat/completions" - background_started = asyncio.Event() - never_release = asyncio.Event() - observed_workloads = [] - - monkeypatch.setenv("ODYSSEUS_LOCAL_MODEL_GATE", "true") - monkeypatch.setenv("BACKGROUND_TASK_FOREGROUND_GATE", "false") - monkeypatch.setattr(llm_core, "_LOCAL_MODEL_LOCK", asyncio.Lock()) - monkeypatch.setattr(llm_core, "_LOCAL_MODEL_CURRENT", {}) - monkeypatch.setattr(llm_core, "_LOCAL_MODEL_WAITING_FOREGROUND", 0) - monkeypatch.setattr( - task_endpoint, - "resolve_task_candidates", - lambda **_kwargs: [(local_url, "scheduled-model", {})], - ) - - async def fake_wait_for_interactive_quiet(_label): - return False - - async def gated_llm_call(url, model, messages, **kwargs): - assert messages - workload = kwargs.get("workload") - observed_workloads.append(workload) - async with llm_core._local_model_slot(url, model, workload=workload): - background_started.set() - await never_release.wait() - return "unreachable" - - monkeypatch.setattr(task_endpoint, "wait_for_interactive_quiet", fake_wait_for_interactive_quiet) - monkeypatch.setattr(llm_core, "llm_call_async", gated_llm_call) - - background_task = asyncio.create_task(email_helpers._generate_scheduled_email_summary( - url=local_url, - model="scheduled-model", - sender="Sender", - subject="Scheduled", - body_for_llm="Scheduled body", - owner="alice", - )) - foreground_task = None - try: - await asyncio.wait_for(background_started.wait(), timeout=1) - - async def run_foreground(): - async with llm_core._local_model_slot( - local_url, - "interactive-model", - workload="foreground", - ): - return True - - foreground_task = asyncio.create_task(run_foreground()) - with pytest.raises(asyncio.CancelledError): - await asyncio.wait_for(background_task, timeout=1) - assert await asyncio.wait_for(foreground_task, timeout=1) is True - assert observed_workloads == ["background"] - finally: - for task in (background_task, foreground_task): - if task is not None and not task.done(): - task.cancel() - - -@pytest.mark.asyncio -async def test_manual_email_summary_uses_shared_helper_and_caches(tmp_path, monkeypatch): - import routes.email_helpers as email_helpers - import routes.email_routes as email_routes - import src.endpoint_resolver as endpoint_resolver - - db_path = tmp_path / "scheduled_emails.db" - monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path) - monkeypatch.setattr(email_routes, "SCHEDULED_DB", db_path) - email_helpers._init_scheduled_db() - - resolve_calls = [] - - def fake_resolve_endpoint(kind, owner=None): - resolve_calls.append((kind, owner)) - assert kind == "utility" - assert owner == "alice" - return ( - "https://chatgpt.com/backend-api/codex/responses", - "gpt-5.5", - {"Authorization": "Bearer test"}, - ) - - helper_calls = {} - - async def fake_generate_email_summary(**kwargs): - helper_calls.update(kwargs) - return "- Manual summary" - - monkeypatch.setattr(endpoint_resolver, "resolve_endpoint", fake_resolve_endpoint) - monkeypatch.setattr(email_routes, "_generate_email_summary", fake_generate_email_summary) - - router = email_routes.setup_email_routes() - summarize = _route_endpoint(router, "/api/email/summarize", "POST") - - result = await summarize( - { - "body": "This is a long enough email body for manual summary.", - "subject": "Manual subject", - "from": "Sender ", - "message_id": "", - "folder": "INBOX", - }, - owner="alice", - ) - - assert result == { - "success": True, - "summary": "- Manual summary", - "model_used": "gpt-5.5", - } - assert resolve_calls == [("utility", "alice")] - assert helper_calls["url"] == "https://chatgpt.com/backend-api/codex/responses" - assert helper_calls["model"] == "gpt-5.5" - assert helper_calls["headers"]["Authorization"] == "Bearer test" - assert helper_calls["headers"]["Content-Type"] == "application/json" - - conn = sqlite3.connect(db_path) - try: - row = conn.execute( - "SELECT owner, summary, model_used FROM email_summaries WHERE message_id=?", - ("",), - ).fetchone() - finally: - conn.close() - assert row == ("alice", "- Manual summary", "gpt-5.5") - - -@pytest.mark.asyncio -@pytest.mark.parametrize("exception_kind", ["http", "runtime"]) -async def test_manual_email_summary_never_exposes_provider_exception( - monkeypatch, - caplog, - exception_kind, -): - from fastapi import HTTPException - import routes.email_routes as email_routes - import src.endpoint_resolver as endpoint_resolver - - secret_detail = ( - "endpoint=https://private.example.internal/v1 provider=ollama " - "model=private-model response_body=private-response " - "Authorization: Bearer token-secret-value" - ) - - def fake_resolve_endpoint(kind, owner=None): - assert kind == "utility" - assert owner == "alice" - return ( - "https://private.example.internal/v1", - "private-model", - {"Authorization": "Bearer token-secret-value"}, - ) - - async def fail_summary(**_kwargs): - if exception_kind == "http": - raise HTTPException(status_code=502, detail=secret_detail) - raise RuntimeError(secret_detail) - - monkeypatch.setattr(endpoint_resolver, "resolve_endpoint", fake_resolve_endpoint) - monkeypatch.setattr(email_routes, "_generate_email_summary", fail_summary) - caplog.set_level(logging.WARNING, logger=email_routes.__name__) - - router = email_routes.setup_email_routes() - summarize = _route_endpoint(router, "/api/email/summarize", "POST") - result = await summarize( - { - "body": "This email body is long enough to summarize.", - "subject": "Sensitive provider failure", - "from": "Sender ", - }, - owner="alice", - ) - - assert result == { - "success": False, - "error": "Failed to summarize", - "error_code": "email_summary_unavailable", - } - exposed = json.dumps(result) + caplog.text - for marker in ( - "private.example.internal", - "ollama", - "private-model", - "private-response", - "token-secret-value", - ): - assert marker not in exposed - assert f"type={'HTTPException' if exception_kind == 'http' else 'RuntimeError'}" in caplog.text - - -@pytest.mark.asyncio -async def test_scheduled_email_summary_uses_shared_helper_and_caches(tmp_path, monkeypatch): - import routes.email_helpers as email_helpers - import routes.email_pollers as email_pollers - - db_path = tmp_path / "scheduled_emails.db" - monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path) - monkeypatch.setattr(email_pollers, "SCHEDULED_DB", db_path) - email_helpers._init_scheduled_db() - - raw_email = ( - b"From: Sender \r\n" - b"To: Alice \r\n" - b"Subject: Scheduled subject\r\n" - b"Message-ID: \r\n" - b"Date: Tue, 01 Jan 2026 12:00:00 +0000\r\n" - b"Content-Type: text/plain; charset=utf-8\r\n" - b"\r\n" - + (b"Please review this scheduled summary email. " * 8) - ) - - class FakeImap: - def __init__(self): - self.logout_calls = 0 - - def select(self, _folder, readonly=True): - return "OK", [] - - def uid(self, command, *args): - if command == "SEARCH": - return "OK", [b"1"] - if command == "FETCH": - return "OK", [(b"1 (RFC822)", raw_email)] - raise AssertionError(f"unexpected uid command: {command!r} {args!r}") - - def logout(self): - self.logout_calls += 1 - - fake_conn = FakeImap() - - def fake_resolve_task_candidates(owner=None): - assert owner == "alice" - return [( - "https://chatgpt.com/backend-api/codex/responses", - "gpt-5.5", - {"Authorization": "Bearer test"}, - )] - - helper_calls = {} - - async def fake_generate_email_summary(**kwargs): - helper_calls.update(kwargs) - return "- Scheduled summary" - - monkeypatch.setattr(email_pollers, "_load_settings", lambda: {"email_auto_summarize": True}) - monkeypatch.setattr(email_pollers, "_owner_for_email_account", lambda _account_id: "alice") - monkeypatch.setattr(email_pollers, "_imap_connect", lambda account_id=None, owner="": fake_conn) - monkeypatch.setattr(email_pollers, "_get_email_config", lambda account_id=None, owner="": {"from_address": "alice@example.com"}) - monkeypatch.setattr(email_pollers, "resolve_task_candidates", fake_resolve_task_candidates) - monkeypatch.setattr(email_pollers, "_generate_scheduled_email_summary", fake_generate_email_summary) - - result = await email_pollers._auto_summarize_pass_single(account_id="acct-alice") - - assert "summarized 1" in result - assert "summary failed" not in result - assert helper_calls["url"] == "https://chatgpt.com/backend-api/codex/responses" - assert helper_calls["model"] == "gpt-5.5" - assert helper_calls["headers"]["Authorization"] == "Bearer test" - assert helper_calls["headers"]["Content-Type"] == "application/json" - assert helper_calls["owner"] == "alice" - assert fake_conn.logout_calls == 1 - - conn = sqlite3.connect(db_path) - try: - row = conn.execute( - "SELECT owner, summary, model_used FROM email_summaries WHERE message_id=?", - ("",), - ).fetchone() - finally: - conn.close() - assert row == ("alice", "- Scheduled summary", "gpt-5.5") diff --git a/tests/test_memory_add_submit_regression.py b/tests/test_memory_add_submit_regression.py deleted file mode 100644 index 450d63003..000000000 --- a/tests/test_memory_add_submit_regression.py +++ /dev/null @@ -1,54 +0,0 @@ -"""The Brain > Add Memory form must be submittable (#5828). - -The form previously had no submit button and relied on a deprecated -``keypress`` listener for Enter, which is not guaranteed to fire on all -platforms — leaving the form with no working submit path. Pins: - -- a visible, keyboard-accessible submit button next to the category select; -- the button wired to ``memoryModule.addNewMemory()``; -- Enter handled via ``keydown`` with ``preventDefault()`` (and no lingering - ``keypress`` handler on the input). -""" -from pathlib import Path - -APP_JS = Path("static/app.js") -INDEX_HTML = Path("static/index.html") - - -def _add_memory_row(html): - start = html.index('id="new-memory-input"') - end = html.index("
", html.index('id="new-memory-add-btn"', start)) - return html[start:end] - - -def test_add_memory_form_renders_a_submit_button(): - html = INDEX_HTML.read_text() - row = _add_memory_row(html) - - assert 'id="new-memory-category"' in row, "button must sit in the same row as the form fields" - btn_start = row.index('id="new-memory-add-btn"') - btn_tag = row[row.rindex("", btn_start)] - assert 'type="button"' in btn_tag, "must not rely on implicit submit semantics" - - -def _new_memory_wiring_block(source): - start = source.index("const newMemoryInput = el('new-memory-input');") - end = source.index("// Voice recording", start) - return source[start:end] - - -def test_submit_button_is_wired_to_add_new_memory(): - block = _new_memory_wiring_block(APP_JS.read_text()) - - assert "el('new-memory-add-btn')" in block - assert "addEventListener('click', () => memoryModule.addNewMemory())" in block - - -def test_enter_uses_keydown_with_prevent_default(): - block = _new_memory_wiring_block(APP_JS.read_text()) - - assert "addEventListener('keydown'" in block - assert "addEventListener('keypress'" not in block, "keypress is deprecated and unreliable for Enter" - assert "e.preventDefault();" in block - assert "!e.isComposing" in block, "IME composition must not submit the form" - assert "memoryModule.addNewMemory();" in block diff --git a/tests/test_tool_parsing_bare_end_marker.py b/tests/test_tool_parsing_bare_end_marker.py deleted file mode 100644 index 6167c8dde..000000000 --- a/tests/test_tool_parsing_bare_end_marker.py +++ /dev/null @@ -1,96 +0,0 @@ -"""Regression: the Qwen bare-marker scrub must not eat a lone `end` (#5547). - -`_QWEN_BARE_MARKER_RE` cleans Qwen turn markers that leak into content. Its -`end` branch was `\\|?end\\|?` — both pipes optional — so it also matched a bare -`end` surrounded by whitespace and replaced it with a space. Any message -containing Ruby, Lua or shell code that closes a block with a lone `end` had -those lines silently deleted, in the stored text and in the rendered message. - -Requiring at least one pipe keeps every real marker (`|end`, `end|`, `|end|`, -`/|end|`) stripping as before. The same pattern is duplicated in -static/js/chatRenderer.js, so the JS copy is checked here too — the two must -not drift. -""" -import json -import re -import shutil -import subprocess -from pathlib import Path - -import pytest - -import src.agent_tools # noqa: F401 (break agent_tools<->tool_parsing import cycle) -from src.tool_parsing import strip_tool_blocks - -_REPO = Path(__file__).resolve().parent.parent -_CHAT_RENDERER = _REPO / "static" / "js" / "chatRenderer.js" - -# Inputs that must survive untouched, and the substring that proves they did. -KEPT = [ - ("loop do\n puts \"yo\"\nend\n", "\nend"), # the reported Ruby case - ("if x then\nend", "\nend"), - ("function f()\nend\n", "\nend"), - ("a end b", "a end b"), - ("append end", "append end"), - ("END", "END"), - ("\nEnd\n", "End"), -] - -# Real markers — at least one pipe, plus the role word — with the exact output -# they must still produce. Asserted as equality rather than "marker not in out" -# so narrowing the pattern can't pass by deleting more than it should. -STRIPPED = [ - ("a |end| b", "a b"), - ("a /|end| b", "a b"), - ("a |end b", "a b"), - ("a end| b", "a b"), - ("x assistant y", "x y"), -] - - -@pytest.mark.parametrize("text,kept", KEPT) -def test_bare_end_survives_stripping(text, kept): - assert kept in strip_tool_blocks(text) - - -@pytest.mark.parametrize("text,expected", STRIPPED) -def test_piped_end_markers_are_still_stripped(text, expected): - assert strip_tool_blocks(text) == expected - - -def test_bare_end_inside_a_fenced_block_survives(): - """The scrub runs over the whole message, fenced regions included.""" - out = strip_tool_blocks("Here:\n```ruby\nloop do\n puts 1\nend\n```\nDone.") - assert "\nend\n" in out - - -def _js_bare_marker_regex_source(): - src = _CHAT_RENDERER.read_text(encoding="utf-8") - m = re.search(r"^const QWEN_BARE_MARKER_RE = (/.*/[gimsuy]*);$", src, re.MULTILINE) - assert m, "QWEN_BARE_MARKER_RE literal not found in chatRenderer.js" - return m.group(1) - - -def test_js_copy_of_the_pattern_matches_the_python_one(): - """Guard the duplication: the JS branch must require a pipe too.""" - if shutil.which("node") is None: - pytest.skip("node binary not on PATH") - - cases = [text for text, _ in KEPT] + [text for text, _ in STRIPPED] - script = ( - "const RE = %s;\n" - "const cases = JSON.parse(process.argv[1]);\n" - "console.log(JSON.stringify(cases.map(c => c.replace(RE, ' '))));" - % _js_bare_marker_regex_source() - ) - result = subprocess.run( - ["node", "--input-type=module", "-e", script, json.dumps(cases)], - cwd=_REPO, capture_output=True, timeout=15, text=True, - ) - assert result.returncode == 0, f"node failed:\n{result.stderr}" - got = json.loads(result.stdout.splitlines()[-1]) - - for (text, kept), out in zip(KEPT, got): - assert kept in out, f"JS regex dropped {kept!r} from {text!r}" - for (text, expected), out in zip(STRIPPED, got[len(KEPT):]): - assert out == expected, f"JS regex: {text!r} -> {out!r}, expected {expected!r}"