diff --git a/CHANGELOG.md b/CHANGELOG.md index b6df1a29..3b08fdfc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ### Fixed -- **PR #2234** by @Jordan-SkyLF (refines #2207) — Three update-banner improvements: (1) Update summaries no longer repeat the same bullet under both "What you'll notice" and "Worth knowing" — visible notice items keep priority, and the secondary section is omitted when there is no distinct detail to show. (2) Update summaries now cap large commit lists (24 + probe item) before sending them to the summarizer and disclose when the summary uses only the latest commit subjects, while keeping the full comparison link available — bounds summarizer cost on large update ranges while remaining honest about coverage. (3) Update banners now wrap generated-summary links and long update text on narrow/mobile screens inside the banner instead of pushing controls off-screen. 108-line regression coverage for short-target dedup, repeated Agent-summary bullets, large-range capped input, and responsive wrapping. +- **PR #2234** by @Jordan-SkyLF (follow-up to the v0.51.61 update-banner cleanup, refines #2207) — Generated update summaries now preserve all explicit `Notice:` / `Worth knowing:` bullets, deduplicate categorized bullets, prefer the configured `auxiliary.update_summary` text model with main-model fallback, disclose when large ranges are summarized from the latest 24 commit subjects, and constrain long summary panels with visible scroll behavior. Regression coverage pins repeated Agent-summary bullets, all categorized bullets, large-range capped input/disclosure, auxiliary-model routing, and scrollable long-summary panels. - **PR #2236** by @jasonjcwu — Silent failure detection in `api/streaming.py` now scans only NEW messages, not the full conversation history. Pre-fix, the `_assistant_added` check at `_run_agent_streaming` scanned all messages in `result["messages"]` (including pre-turn history); if any prior turn contained an assistant response, `_assistant_added` was `True` and the apperror SSE event was silently skipped, leaving the user staring at a blank response after a provider 401/429/rate-limit error. Fix extracts a `_has_new_assistant_reply(all_messages, prev_count)` helper that only inspects messages beyond the pre-turn history offset (`_previous_context_messages`); applied to both the main detection path and the self-heal/retry `_heal_ok` check. 15-test regression suite covering empty/short/long-history scenarios, the heal path, and the `len < prev_count` edge-case fallback. Also includes a small alignment fix to `test_issue1857_usage_overwrite.py` so the FakeAgent message shape matches what the real agent produces. diff --git a/api/routes.py b/api/routes.py index dadc17f4..e1b53ee1 100644 --- a/api/routes.py +++ b/api/routes.py @@ -5312,41 +5312,71 @@ def handle_post(handler, parsed) -> bool: target = body.get("target") if isinstance(body, dict) else None def _llm_update_summary(system_prompt: str, user_prompt: str) -> str: - from run_agent import AIAgent from api.config import ( get_effective_default_model, resolve_model_provider, resolve_custom_provider_connection, ) - _model, _provider, _base_url = resolve_model_provider(get_effective_default_model()) - _api_key = None + messages = [ + {"role": "system", "content": system_prompt}, + {"role": "user", "content": user_prompt}, + ] + + _main_model, _main_provider, _main_base_url = resolve_model_provider(get_effective_default_model()) + _main_api_key = None try: from api.oauth import resolve_runtime_provider_with_anthropic_env_lock from hermes_cli.runtime_provider import resolve_runtime_provider _rt = resolve_runtime_provider_with_anthropic_env_lock( resolve_runtime_provider, - requested=_provider, + requested=_main_provider, ) - _api_key = _rt.get("api_key") - if not _provider: - _provider = _rt.get("provider") - if not _base_url: - _base_url = _rt.get("base_url") + _main_api_key = _rt.get("api_key") + if not _main_provider: + _main_provider = _rt.get("provider") + if not _main_base_url: + _main_base_url = _rt.get("base_url") except Exception as _e: logger.debug("update summary runtime provider resolution failed: %s", _e) - if isinstance(_provider, str) and _provider.startswith("custom:"): - _cp_key, _cp_base = resolve_custom_provider_connection(_provider) - if not _api_key and _cp_key: - _api_key = _cp_key - if not _base_url and _cp_base: - _base_url = _cp_base + if isinstance(_main_provider, str) and _main_provider.startswith("custom:"): + _cp_key, _cp_base = resolve_custom_provider_connection(_main_provider) + if not _main_api_key and _cp_key: + _main_api_key = _cp_key + if not _main_base_url and _cp_base: + _main_base_url = _cp_base + + main_runtime = { + "provider": _main_provider, + "model": _main_model, + "base_url": _main_base_url, + "api_key": _main_api_key, + } + + try: + from agent.auxiliary_client import get_text_auxiliary_client + + aux_client, aux_model = get_text_auxiliary_client( + "update_summary", + main_runtime=main_runtime, + ) + if aux_client is not None and aux_model: + response = aux_client.chat.completions.create( + model=aux_model, + messages=messages, + ) + return str(response.choices[0].message.content or "").strip() + except Exception as _e: + logger.debug("update summary auxiliary model failed; falling back to main model: %s", _e) + + from run_agent import AIAgent + agent = AIAgent( - model=_model, - provider=_provider, - base_url=_base_url, - api_key=_api_key, + model=_main_model, + provider=_main_provider, + base_url=_main_base_url, + api_key=_main_api_key, platform="webui", quiet_mode=True, enabled_toolsets=[], diff --git a/api/updates.py b/api/updates.py index cbbbfd8e..92007749 100644 --- a/api/updates.py +++ b/api/updates.py @@ -369,11 +369,34 @@ def _clean_summary_bullet(line: str) -> str: return line[:240] +def _split_summary_category(line: str) -> tuple[str | None, str]: + raw = str(line or '').strip() + match = re.match(r'^\s*(?:[-*•]+|\d+[.)])?\s*(notice|what you(?:ll|\'ll| will) notice|user(?:s)? will notice|worth knowing|worth|note)\s*:\s*(.+)$', raw, re.I) + if not match: + return None, raw + label = match.group(1).lower() + category = 'worth' if label in {'worth knowing', 'worth', 'note'} else 'notice' + return category, match.group(2) + + +def _unique_summary_bullets(items: list[str]) -> list[str]: + seen = set() + bullets = [] + for item in items: + cleaned = _clean_summary_bullet(item) + key = cleaned.lower() + if cleaned and key not in seen: + bullets.append(cleaned) + seen.add(key) + return bullets + + def _summary_bullets_from_text(text: str, *, fallback_items: list[str]) -> list[str]: raw = str(text or '').strip() candidates = [] for line in raw.splitlines(): - cleaned = _clean_summary_bullet(line) + _category, body = _split_summary_category(line) + cleaned = _clean_summary_bullet(body) if cleaned: candidates.append(cleaned) if len(candidates) <= 1 and raw: @@ -381,18 +404,22 @@ def _summary_bullets_from_text(text: str, *, fallback_items: list[str]) -> list[ candidates = [item for item in candidates if item] if not candidates: candidates = [_clean_summary_bullet(item) for item in fallback_items] - seen = set() - bullets = [] - for item in candidates: - key = item.lower() - if item and key not in seen: - bullets.append(item) - seen.add(key) - if len(bullets) >= 5: - break + bullets = _unique_summary_bullets(candidates) return bullets or ['Updates are available.'] +def _categorized_summary_bullets_from_text(text: str) -> tuple[list[str], list[str]]: + notice_items: list[str] = [] + worth_items: list[str] = [] + for line in str(text or '').splitlines(): + category, body = _split_summary_category(line) + if category == 'notice': + notice_items.append(body) + elif category == 'worth': + worth_items.append(body) + return _unique_summary_bullets(notice_items), _unique_summary_bullets(worth_items) + + def _fallback_update_bullets(details: list[dict]) -> list[str]: bullets = [] for item in details: @@ -431,21 +458,15 @@ def _worth_knowing_bullets(details: list[dict]) -> list[str]: def _format_update_summary_sections(summary_text: str, details: list[dict]) -> tuple[list[dict], str]: - bullets = _summary_bullets_from_text(summary_text, fallback_items=_fallback_update_bullets(details)) - if len(bullets) > 1: - notice_items = bullets[:3] - worth_items = bullets[3:] - else: - notice_items = bullets - worth_items = [] + notice_items, worth_items = _categorized_summary_bullets_from_text(summary_text) + if not notice_items: + notice_items = _summary_bullets_from_text(summary_text, fallback_items=_fallback_update_bullets(details)) notice_keys = {item.lower() for item in notice_items} worth_items = [item for item in worth_items if item.lower() not in notice_keys] - if not worth_items: - worth_items = [ - item for item in _worth_knowing_bullets(details) - if item.lower() not in notice_keys - ] - worth_items = worth_items[:2] + worth_items.extend( + item for item in _worth_knowing_bullets(details) + if item.lower() not in notice_keys and item.lower() not in {existing.lower() for existing in worth_items} + ) sections = [ { 'title': "What you'll notice", @@ -480,9 +501,12 @@ def _update_summary_prompt(details: list[dict]) -> tuple[str, str]: "Return only bullets. Do not include headings, markdown tables, intro paragraphs, or closing notes." ) user_lines = [ - "Summarize these available updates as 3-5 concise bullets.", + "Summarize these available updates as concise bullets.", + "Prefix each bullet with `Notice:` for user-visible behavior changes or `Worth knowing:` for useful context.", + "Put user-visible Notice bullets first and include every meaningful user-facing change from the available commit subjects.", + "Use Worth knowing only for helpful context that is not a duplicate of a Notice bullet.", "Use everyday language and explain visible behavior changes, not code mechanics.", - "Return only bullets; the WebUI will add the fixed section headings separately.", + "Return only prefixed bullets; the WebUI will add the fixed section headings separately.", "", ] for item in details: diff --git a/docs/pr-media/update-summary-category-dedup/duplicate-summary-after.png b/docs/pr-media/update-summary-category-dedup/duplicate-summary-after.png index 7eb167d0..0e964a84 100644 Binary files a/docs/pr-media/update-summary-category-dedup/duplicate-summary-after.png and b/docs/pr-media/update-summary-category-dedup/duplicate-summary-after.png differ diff --git a/docs/pr-media/update-summary-category-dedup/duplicate-summary-before.png b/docs/pr-media/update-summary-category-dedup/duplicate-summary-before.png index ed1aecb2..5c530582 100644 Binary files a/docs/pr-media/update-summary-category-dedup/duplicate-summary-before.png and b/docs/pr-media/update-summary-category-dedup/duplicate-summary-before.png differ diff --git a/docs/pr-media/update-summary-category-dedup/large-range-after.png b/docs/pr-media/update-summary-category-dedup/large-range-after.png index 3e89d64d..af46274a 100644 Binary files a/docs/pr-media/update-summary-category-dedup/large-range-after.png and b/docs/pr-media/update-summary-category-dedup/large-range-after.png differ diff --git a/docs/pr-media/update-summary-category-dedup/large-range-before.png b/docs/pr-media/update-summary-category-dedup/large-range-before.png index ced6ba89..ad9b13ed 100644 Binary files a/docs/pr-media/update-summary-category-dedup/large-range-before.png and b/docs/pr-media/update-summary-category-dedup/large-range-before.png differ diff --git a/static/style.css b/static/style.css index 2874289d..88ec8a5e 100644 --- a/static/style.css +++ b/static/style.css @@ -571,6 +571,7 @@ .update-banner{display:none;background:var(--surface);border:1px solid var(--accent);border-radius:10px;padding:10px 16px;margin:10px auto;max-width:780px;width:calc(100% - 24px);box-sizing:border-box;font-size:13px;color:var(--accent-text);align-items:flex-start;justify-content:space-between;gap:12px;flex-wrap:wrap;overflow-wrap:anywhere;} .update-banner.visible{display:flex;} #updateMsg,#updateWhatsNewLinks,#updateSummaryPanel,#updateSummaryDiffLinks{max-width:100%;overflow-wrap:anywhere;word-break:break-word;} + #updateSummaryPanel{max-height:min(34vh,260px);overflow:auto;overscroll-behavior:contain;scrollbar-gutter:stable;scrollbar-width:thin;scrollbar-color:var(--accent) transparent;} #updateWhatsNewLinks{white-space:normal!important;margin-left:0!important;} .update-banner>div:last-child{margin-left:auto;} @media (max-width:600px){.update-banner{width:calc(100% - 16px);padding:10px 12px;gap:10px;}.update-banner>div:last-child{width:100%;justify-content:flex-end;}} diff --git a/tests/test_update_banner_fixes.py b/tests/test_update_banner_fixes.py index 90834fc1..6b35daee 100644 --- a/tests/test_update_banner_fixes.py +++ b/tests/test_update_banner_fixes.py @@ -418,7 +418,18 @@ class TestForceUpdateRoute: ) -# ── static/ui.js ────────────────────────────────────────────────────────────── +class TestUpdateSummaryRouteModelSelection: + """Update summaries should use the auxiliary update-summary model before main model fallback.""" + + def test_summary_route_prefers_update_summary_auxiliary_model(self): + src = read('api/routes.py') + + assert 'get_text_auxiliary_client' in src + assert '"update_summary"' in src + assert 'main_runtime=main_runtime' in src + assert 'update summary auxiliary model failed; falling back to main model' in src + assert 'from run_agent import AIAgent' in src + class TestUiJsUpdateBanner: """#813 + #814 — UI must show persistent error, force button, and correct toast.""" @@ -792,7 +803,7 @@ class TestWhatsNewSummaryToggle: assert 'human-readable' in updates assert 'avoid technical jargon' in updates assert 'regular diff comparison' in updates - assert 'Return only bullets' in updates + assert 'Return only prefixed bullets' in updates assert 'def _format_update_summary_sections' in updates def test_update_summary_formats_llm_text_into_stable_sections(self): @@ -875,6 +886,43 @@ class TestWhatsNewSummaryToggle: assert result['summary'].count(duplicate_menu_item) == 1 assert result['summary'].count(duplicate_quality_item) == 1 + def test_update_summary_keeps_all_categorized_notice_and_worth_bullets(self): + from api.updates import summarize_update_payload + + result = summarize_update_payload( + {'webui': {'behind': 8, 'current_sha': 'abc', 'latest_sha': 'def', 'compare_url': 'https://example.test/webui'}}, + llm_callback=lambda _system, _prompt: '\n'.join( + [ + 'Notice: The settings panel loads faster.', + 'Notice: Update prompts are easier to read.', + 'Notice: Chat status is clearer during reconnects.', + 'Notice: Tool results stay grouped by source.', + 'Notice: Mobile controls remain visible.', + 'Worth knowing: Some labels were renamed to match the new flow.', + 'Worth knowing: The full diff is still available from the update banner.', + ] + ), + use_cache=False, + ) + sections = {section['title']: section['items'] for section in result['summary_sections']} + + assert sections["What you'll notice"] == [ + 'The settings panel loads faster.', + 'Update prompts are easier to read.', + 'Chat status is clearer during reconnects.', + 'Tool results stay grouped by source.', + 'Mobile controls remain visible.', + ] + assert sections['Worth knowing'] == [ + 'Some labels were renamed to match the new flow.', + 'The full diff is still available from the update banner.', + ] + + def test_update_summary_panel_is_scrollable_for_long_summaries(self): + style = read('static/style.css') + + assert '#updateSummaryPanel{max-height:min(34vh,260px);overflow:auto;overscroll-behavior:contain;scrollbar-gutter:stable;scrollbar-width:thin;scrollbar-color:var(--accent) transparent;}' in style + def test_update_summary_many_updates_caps_commit_input_and_discloses_scope(self, monkeypatch): import api.updates as upd @@ -889,9 +937,11 @@ class TestWhatsNewSummaryToggle: def fake_llm(_system, prompt): prompts.append(prompt) return '\n'.join([ - 'Several user-facing fixes are ready.', - 'Settings and update messaging should be easier to understand.', - 'The update flow should feel safer and clearer.', + 'Notice: Several user-facing fixes are ready.', + 'Notice: Settings and update messaging should be easier to understand.', + 'Notice: The update flow should feel safer and clearer.', + 'Notice: Mobile update controls should stay reachable.', + 'Worth knowing: Some lower-level cleanup supports the visible update changes.', ]) result = upd.summarize_update_payload( @@ -918,9 +968,11 @@ class TestWhatsNewSummaryToggle: 'Several user-facing fixes are ready.', 'Settings and update messaging should be easier to understand.', 'The update flow should feel safer and clearer.', + 'Mobile update controls should stay reachable.', ] assert sections['Worth knowing'] == [ - 'WebUI has 57 updates; this summary uses the latest 24 commit subjects, with the full comparison still available in the diff link.' + 'Some lower-level cleanup supports the visible update changes.', + 'WebUI has 57 updates; this summary uses the latest 24 commit subjects, with the full comparison still available in the diff link.', ] assert result['targets'][0]['commits_truncated'] is True