From 80c0d1a2f4a0ce79896bafef8bffe41c91dcbbc4 Mon Sep 17 00:00:00 2001 From: kokhlo Date: Mon, 28 Sep 2026 14:03:48 +0300 Subject: [PATCH] fix(bots): key the Bot Chat history wait on paintable rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A bot whose canonical Bot Chat session has message_count > 0 but zero paintable rows can never be opened from the roster: the click demands a painted transcript (expectHistory), no read mode serves active = 0, compacted = 0 rows, so the open burns the full 60s hydration budget and fails closed, and the fail-closed registry guard then refuses to mint a replacement either. Report the paintable size next to the denormalized total: - session.list title lookups and profiles.list canonical/last_session rows now carry live_message_count — the count of display-set rows (active = 1 OR compacted = 1) a stored-transcript read actually paints, distinct display groups counted once. - the desktop keys expectHistory off live_message_count when present, falling back to message_count for older gateways, so the count and the paint gate can no longer disagree. An all-folded chat opens as expected-empty instead of wedging for the whole budget. (cherry picked from commit ddee47676f5f0669f7cb84681dc2f6c4a72ee4c6) --- .../canonical-chat-expect-history.test.ts | 33 +++++++++ .../src/plugins/hermes-bots/canonical-chat.ts | 29 +++++++- .../hermes-bots/recent-session.test.ts | 6 +- .../src/plugins/hermes-bots/recent-session.ts | 4 +- apps/desktop/src/plugins/hermes-bots/types.ts | 2 + .../test_profiles_list_canonical_session.py | 69 +++++++++++++++++++ tui_gateway/methods_profiles.py | 16 ++++- tui_gateway/methods_session.py | 44 ++++++++++-- 8 files changed, 189 insertions(+), 14 deletions(-) create mode 100644 apps/desktop/src/plugins/hermes-bots/canonical-chat-expect-history.test.ts diff --git a/apps/desktop/src/plugins/hermes-bots/canonical-chat-expect-history.test.ts b/apps/desktop/src/plugins/hermes-bots/canonical-chat-expect-history.test.ts new file mode 100644 index 0000000000..121633d30a --- /dev/null +++ b/apps/desktop/src/plugins/hermes-bots/canonical-chat-expect-history.test.ts @@ -0,0 +1,33 @@ +/** + * The roster's history wait must key off rows a transcript read can paint + * (`live_message_count`), not the denormalized `message_count` total: a chat + * whose rows are all folded advertises history no reader serves, and waiting + * for it wedges the open for the whole hydration budget before failing closed. + */ + +import { describe, expect, it } from 'vitest' + +import { resolveExpectHistory } from './canonical-chat' + +describe('resolveExpectHistory', () => { + it('waits for history only when a transcript read can paint rows', () => { + expect(resolveExpectHistory({ live_message_count: 2, message_count: 2 })).toBe(true) + expect(resolveExpectHistory({ live_message_count: 0, message_count: 2 })).toBe(false) + }) + + it('falls back to the denormalized total on older gateways', () => { + expect(resolveExpectHistory({ message_count: 3 })).toBe(true) + expect(resolveExpectHistory({ message_count: 0 })).toBe(false) + }) + + it('waits when no count is reported at all', () => { + expect(resolveExpectHistory(null)).toBe(true) + expect(resolveExpectHistory(undefined)).toBe(true) + expect(resolveExpectHistory({})).toBe(true) + }) + + it('ignores non-finite counts the same way the open path always has', () => { + expect(resolveExpectHistory({ live_message_count: Number.NaN, message_count: 1 })).toBe(true) + expect(resolveExpectHistory({ message_count: Number.NaN })).toBe(true) + }) +}) diff --git a/apps/desktop/src/plugins/hermes-bots/canonical-chat.ts b/apps/desktop/src/plugins/hermes-bots/canonical-chat.ts index 04e8dfea28..59005111eb 100644 --- a/apps/desktop/src/plugins/hermes-bots/canonical-chat.ts +++ b/apps/desktop/src/plugins/hermes-bots/canonical-chat.ts @@ -55,6 +55,28 @@ export const CANONICAL_CHAT_TITLE = 'Bot Chat' * properties. */ interface CanonicalChatRow extends CanonicalSession { readonly message_count?: number + /** Rows a stored-transcript read can paint (active + compaction-archived); + * absent on older gateways, which only report the denormalized total. */ + readonly live_message_count?: number +} + +/** Should the open wait for a painted transcript? The paintable row count + * decides when the gateway reports it; the denormalized total is the only + * fallback older gateways offer, and no count at all means the row is + * guesswork anyway — wait, so an empty paint still surfaces as an error + * instead of a silent blank chat. */ +export function resolveExpectHistory( + summary: null | undefined | { message_count?: number; live_message_count?: number } +): boolean { + if (typeof summary?.live_message_count === 'number' && Number.isFinite(summary.live_message_count)) { + return summary.live_message_count > 0 + } + + if (typeof summary?.message_count === 'number' && Number.isFinite(summary.message_count)) { + return summary.message_count > 0 + } + + return true } /** Is the chat on screen the given bot's forever-chat? @@ -99,8 +121,11 @@ async function openStoredBotChat( const { bot, name, route } = botOwner(owner) const ownerKey = botWorkspaceOwnerKey(bot) - const hasAuthoritativeCount = typeof summary?.message_count === 'number' && Number.isFinite(summary.message_count) - const expectHistory = hasAuthoritativeCount ? summary.message_count > 0 : true + // Paintable rows decide the wait, not the denormalized total: a chat whose rows + // are all folded (orphaned compaction marks, a full rewind) paints nothing, and + // demanding its history wedges the open for the whole hydration budget before + // failing closed. Older gateways report only the total — keep trusting it there. + const expectHistory = resolveExpectHistory(summary) // Current SDKs export the Bot-specific budget. The fallback preserves // compatibility with older hosts and isolated plugin test harnesses. diff --git a/apps/desktop/src/plugins/hermes-bots/recent-session.test.ts b/apps/desktop/src/plugins/hermes-bots/recent-session.test.ts index 2c2352e274..7d07e77e12 100644 --- a/apps/desktop/src/plugins/hermes-bots/recent-session.test.ts +++ b/apps/desktop/src/plugins/hermes-bots/recent-session.test.ts @@ -19,7 +19,11 @@ vi.mock('@hermes/plugin-sdk', () => ({ host: { openSession, notifyError: vi.fn() } })) vi.mock('./bot-state', () => ({ saveSelectedRosterBot: () => undefined })) -vi.mock('./canonical-chat', () => ({ prepareBotSource })) +vi.mock('./canonical-chat', () => ({ + prepareBotSource, + resolveExpectHistory: (summary: { message_count?: number; live_message_count?: number } | null | undefined) => + (summary?.live_message_count ?? summary?.message_count ?? 1) > 0 +})) vi.mock('./roster-actions', () => ({ openRosterBot })) vi.mock('./routing', () => ({ botConnectionRoute: () => ({ connectionId: 'local', mode: 'local', profile: 'researcher' }), diff --git a/apps/desktop/src/plugins/hermes-bots/recent-session.ts b/apps/desktop/src/plugins/hermes-bots/recent-session.ts index a9eaa51e2d..3ad3c244d6 100644 --- a/apps/desktop/src/plugins/hermes-bots/recent-session.ts +++ b/apps/desktop/src/plugins/hermes-bots/recent-session.ts @@ -16,7 +16,7 @@ import * as sdk from '@hermes/plugin-sdk' import { haptic, host } from '@hermes/plugin-sdk' import { saveSelectedRosterBot } from './bot-state' -import { prepareBotSource } from './canonical-chat' +import { prepareBotSource, resolveExpectHistory } from './canonical-chat' import { openRosterBot } from './roster-actions' import { botConnectionRoute, botWorkspaceOwnerKey, setBotsWorkspaceOwner } from './routing' import type { RosterRow } from './types' @@ -55,7 +55,7 @@ export async function openBotRecentSession(bot: RosterRow): Promise { // stays where the row click expects it (same shape as `+` side threads). intent: 'tab', awaitHydration: true, - expectHistory: (bot.last_session?.message_count ?? 1) > 0, + expectHistory: resolveExpectHistory(bot.last_session), forceResume: true, hydrationTimeoutMs: Number.isFinite(sdk.BOT_CHAT_SESSION_HYDRATION_TIMEOUT_MS) ? sdk.BOT_CHAT_SESSION_HYDRATION_TIMEOUT_MS diff --git a/apps/desktop/src/plugins/hermes-bots/types.ts b/apps/desktop/src/plugins/hermes-bots/types.ts index 2fdfea8871..92c6571108 100644 --- a/apps/desktop/src/plugins/hermes-bots/types.ts +++ b/apps/desktop/src/plugins/hermes-bots/types.ts @@ -53,6 +53,8 @@ export interface SessionPreview { /** Unix seconds, not milliseconds. */ last_active?: number message_count?: number + /** Rows a stored-transcript read can paint; absent on older gateways. */ + live_message_count?: number preview?: string title?: string } diff --git a/tests/tui_gateway/test_profiles_list_canonical_session.py b/tests/tui_gateway/test_profiles_list_canonical_session.py index 17551d0148..df8490079b 100644 --- a/tests/tui_gateway/test_profiles_list_canonical_session.py +++ b/tests/tui_gateway/test_profiles_list_canonical_session.py @@ -194,6 +194,75 @@ def test_canonical_session_ignores_unmarked_normal_child(home): assert "canonical bot content" in canonical["preview"] +# --------------------------------------------------------------------------- +# live_message_count: the count the roster's history wait can trust +# --------------------------------------------------------------------------- + + +def _fold_all_rows(db, sid): + """Fold every row of one session the way an orphaned compaction mark does.""" + with db._lock: + db._conn.execute( + "UPDATE messages SET active = 0, _compressed_summary = 1 WHERE session_id = ?", (sid,)) + + +def test_canonical_session_live_count_zero_when_all_rows_folded(home): + db = _db(home) + _add_session(db, "foldedchat", title="Bot Chat", ts=1000, + text="folded away", hidden=True) + _fold_all_rows(db, "foldedchat") + db.close() + + canonical = _row(_profiles({}), "default")["canonical_session"] + + # The denormalized total still advertises history no reader can serve... + assert canonical["message_count"] > 0 + # ...while the paintable count says the open must not wait for a transcript. + assert canonical["live_message_count"] == 0 + + +def test_canonical_session_live_count_counts_paintable_rows(home): + db = _db(home) + _add_session(db, "livechat", title="Bot Chat", ts=1000, + text="still painted", hidden=True) + db.close() + + canonical = _row(_profiles({}), "default")["canonical_session"] + + assert canonical["message_count"] == 1 + assert canonical["live_message_count"] == 1 + + +def test_session_list_title_lookup_reports_live_count(home, monkeypatch): + # The click path's exact-title lookup carries the same field. + db = _db(home) + _add_session(db, "folded2", title="Bot Chat", ts=1000, + text="click target", hidden=True) + _fold_all_rows(db, "folded2") + monkeypatch.setattr(srv, "_get_db", lambda: db) + + envelope = srv._methods["session.list"](1, {"title": "Bot Chat"}) + sessions = envelope["result"]["sessions"] + + assert len(sessions) == 1 + assert sessions[0]["message_count"] > 0 + assert sessions[0]["live_message_count"] == 0 + db.close() + + +def test_profiles_list_last_session_reports_live_count(home): + db = _db(home) + _add_session(db, "visible", title="Scratch", ts=1000, text="ordinary content") + _fold_all_rows(db, "visible") + db.close() + + row = _row(_profiles({}), "default") + + assert row["last_session"]["id"] == "visible" + assert row["last_session"]["message_count"] > 0 + assert row["last_session"]["live_message_count"] == 0 + + # --------------------------------------------------------------------------- # Recoverable-archive resurrection (#92687) # --------------------------------------------------------------------------- diff --git a/tui_gateway/methods_profiles.py b/tui_gateway/methods_profiles.py index d7fce74e65..afd8092b01 100644 --- a/tui_gateway/methods_profiles.py +++ b/tui_gateway/methods_profiles.py @@ -145,6 +145,12 @@ def _resurrect_recoverable_canonical(db, profile_path, session_id): return False +def _live_count_field(db, session_id) -> dict: + """``{"live_message_count": n}`` for the row a stored-transcript read can paint, else ``{}``.""" + live = _display_row_count(db, session_id) + return {} if live is None else {"live_message_count": live} + + def _canonical_session_row(db, profile_path): """Summary of the profile's canonical "Bot Chat" row (identity is the NAME), or None. Lineages via ``get_compression_tip`` (NOT the resume walker's unmarked-child fallback); @@ -172,12 +178,17 @@ def _canonical_session_row(db, profile_path): tip = _try(lambda: db.get_compression_tip(session_id), None) or session_id tip_row = db.get_session(tip) or row started = row.get("started_at") or 0 + # live_message_count sizes what a stored-transcript read can actually paint; the + # denormalized message_count counts folded rows too (orphaned compaction marks, + # full rewinds) and once made the roster demand history no reader serves. + live = _display_row_count(db, tip) return { "id": session_id, "resolved_id": tip, "root_title": row.get("title") or "", "title": tip_row.get("title") or "", "preview": _latest_message_preview(db, tip), "started_at": tip_row.get("started_at") or started, "last_active": tip_row.get("last_activity_at") or tip_row.get("started_at") or started, - "message_count": tip_row.get("message_count") or 0} + "message_count": tip_row.get("message_count") or 0, + **({} if live is None else {"live_message_count": live})} except Exception: return None @@ -206,7 +217,8 @@ def _latest_profile_session_rows(db): human = {"id": s["id"], "title": title, "preview": _latest_message_preview(db, s["id"]) or s.get("preview") or "", "started_at": s.get("started_at") or 0, "last_active": last_active, - "message_count": s.get("message_count") or 0} + "message_count": s.get("message_count") or 0, + **_live_count_field(db, s["id"])} if human is not None and worker is not None: break return human, worker diff --git a/tui_gateway/methods_session.py b/tui_gateway/methods_session.py index 5c54fd0835..3e011e4346 100644 --- a/tui_gateway/methods_session.py +++ b/tui_gateway/methods_session.py @@ -115,13 +115,40 @@ def _cwd_info(session: dict, cwd: str, branch=None) -> dict: "desktop_contract": DESKTOP_BACKEND_CONTRACT} -def _session_row_summary(row: dict, *, tip_row: dict | None = None, resolved_id=None) -> dict: - """Compact session.list row; ``tip_row``/``resolved_id`` come from the compression tip.""" +def _session_row_summary(row: dict, *, tip_row: dict | None = None, resolved_id=None, db=None) -> dict: + """Compact session.list row; ``tip_row``/``resolved_id`` come from the compression tip. + + ``live_message_count`` (present when ``db`` is given) counts exactly the rows a + stored-transcript read can paint — the tip's display set (``active = 1 OR + compacted = 1``). ``message_count`` is the denormalized total and counts folded + rows too, so a session whose rows are all inactive (orphaned compaction marks, + fully rewound chats) advertises history no reader serves.""" tip_row = tip_row or row - return {"id": row["id"], **({} if resolved_id is None else {"resolved_id": resolved_id}), - "title": row.get("title") or "", "preview": tip_row.get("preview") or "", - "started_at": row.get("started_at") or 0, "message_count": tip_row.get("message_count") or 0, - "source": row.get("source") or ""} + summary = {"id": row["id"], **({} if resolved_id is None else {"resolved_id": resolved_id}), + "title": row.get("title") or "", "preview": tip_row.get("preview") or "", + "started_at": row.get("started_at") or 0, "message_count": tip_row.get("message_count") or 0, + "source": row.get("source") or ""} + if db is not None: + live = _display_row_count(db, resolved_id if resolved_id is not None else row["id"]) + if live is not None: + summary["live_message_count"] = live + return summary + + +def _display_row_count(db, session_id) -> int | None: + """Painted-transcript size of one session segment, or None when unreadable (older stores). + + Mirrors ``_display_rows_from_conn``: the read paints one representative per + ``display_order`` group among ``active = 1 OR compacted = 1`` rows (legacy rows + without a display order collapse into one group, matching the legacy page).""" + try: + with db._lock: + return int(db._conn.execute( + "SELECT COUNT(*) FROM (SELECT DISTINCT display_order FROM messages" + " WHERE session_id = ? AND (active = 1 OR compacted = 1))", + (str(session_id),)).fetchone()[0]) + except Exception: + return None from hermes_state_sessions import INTERNAL_LISTING_SOURCES @@ -518,7 +545,7 @@ def _session_list_by_title(rid, db, title_lookup: str) -> dict: # Real compression continuation only: the resolver's unmarked-child fallback could redirect Bot Chat. tip = db.get_compression_tip(row["id"]) or row["id"] tip_row = (db.get_session(tip) or row) if tip != row["id"] else row - return _ok(rid, {"sessions": [_session_row_summary(row, tip_row=tip_row, resolved_id=tip)]}) + return _ok(rid, {"sessions": [_session_row_summary(row, tip_row=tip_row, resolved_id=tip, db=db)]}) @method("session.list") @@ -540,6 +567,9 @@ def _(rid, params: dict, db) -> dict: include_subagents = bool(db_path) and show_subagent_sessions(Path(db_path).parent) rows = _listing_rows(db, max(limit * 2, 200), include_hidden=_flag(params, "include_hidden"), include_subagents=include_subagents)[:limit] + # No live_message_count here on purpose: the bulk listing serves rows whose open + # paths never demand history (expectHistory defaults false); the count is a + # per-session scan and would turn one listing call into hundreds of them. return _ok(rid, {"sessions": [_session_row_summary(s) for s in rows]}) except Exception as e: return _err(rid, 5006, str(e))