fix(bots): key the Bot Chat history wait on paintable rows
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)
This commit is contained in:
committed by
Brooklyn Nicholson
parent
99aa3ca0af
commit
80c0d1a2f4
@@ -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)
|
||||
})
|
||||
})
|
||||
@@ -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.
|
||||
|
||||
@@ -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' }),
|
||||
|
||||
@@ -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<boolean> {
|
||||
// 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
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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))
|
||||
|
||||
Reference in New Issue
Block a user