fix(gateway): stop /refine cold-cache false emptiness

[fangliquanflq]

What does this PR do?

On messaging platforms, /refine no longer treats a cold _agent_cache as an empty conversation. When the live agent is missing, the handler loads the persisted transcript off the event loop and reports that durable turns exist so the user can resume first. When the agent is cached but _session_messages is empty, it falls back to that transcript and starts the review. Transcript read failures return a distinct error instead of looking like emptiness.

Symptom

/refine on Telegram/Discord/etc. can return Nothing to refine yet - send a message first. after completed turns once the idle agent has left _agent_cache.

Impact

Messaging users cannot refine an existing conversation, or get a false empty-conversation claim, even though the session store still has usable turns.

Bug Cause

Trigger: gateway/slash_commands.py::_handle_refine_command with a cold _agent_cache

Causal chain:

1. Messaging /refine looks up the session agent only in _agent_cache.
2. A missing cache entry returns the same "Nothing to refine yet" response used for a genuinely empty conversation.
3. The handler never consults session_store.load_transcript, so durable turns are ignored.

Why it is wrong: Cache absence is not emptiness. The Desktop/TUI path already fixed the sibling false claim in #83520; messaging still had the cold-cache branch.

Working sibling / contrast: #83520 loads persisted history and runs refine for Desktop/TUI. This PR covers the messaging gateway path called out as complementary on #83455.

Ruled out: A truly empty transcript still returns the original empty-conversation messages.

Fix

Resolve the session entry, load the transcript via async_session_store (already offloaded with asyncio.to_thread), count user/assistant rows, and:

• cold cache + N > 0 -> ask to resume/wake first
• transcript read failure -> distinct "couldn't read" answer
• cached agent + empty _session_messages + durable turns -> run review against the persisted snapshot

Related Issue

Fixes #83871

Related: #83455, #83520

Type of Change

• Bug fix

Changes Made

• gateway/slash_commands.py - consult persisted transcript for cold-cache and empty in-memory /refine paths
• tests/gateway/test_refine_command.py - cover resume prompt, true empty, read failure, and persisted-snapshot review

How to Test

scripts/run_tests.sh tests/gateway/test_refine_command.py

Results: 5 passed on Windows with the project venv.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix
• I've run the repo's test entry on relevant tests
• I've added regression tests for this bug fix
• I've tested on Windows

Documentation & Housekeeping

• Documentation update - N/A (behavior fix only)
• cli-config.yaml.example - N/A
• CONTRIBUTING.md / AGENTS.md - N/A
• Cross-platform impact considered (async offload via existing AsyncSessionStore)
• Tool descriptions/schemas - N/A

[Enough1122]

AI code review — automated review for reference, author can ignore or act on any point.

fix(gateway): stop /refine cold-cache false emptiness

1. _load_refine_persisted_transcript uses get_or_create_session — issuing /refine on a brand-new session (no history at all) creates an empty session row as a side effect of what is a read-only intent. A read-only lookup (get the entry if it exists, else treat as empty) would avoid persisting a phantom session; the row should be created on the actual resume/message path instead.
2. _count_refine_persisted_turns counts only user/assistant rows across the whole transcript, but the message says "N persisted messages" — for tool-heavy sessions the number understates the actual message count. Consider wording like "N persisted user/assistant messages" to avoid confusion.
3. When the agent is cached but _session_messages is empty, the persisted transcript becomes the review snapshot directly — persisted rows may carry extra keys (ids, timestamps, tool metadata) that _spawn_background_review's snapshot handling may not expect. The tests cover role/content only; normalizing the snapshot before passing it downstream would harden this path.

[fangliquanflq]

Thanks for the detailed review. I investigated each point and pushed the actionable fixes in faf5382b3:

1. Read-only lookup: fixed. /refine now resolves the existing routing entry with the public, lock-held peek_session_id() accessor. An unknown session key returns the true-empty response without creating a session row or attempting a transcript load.
2. Count wording: fixed. The response now says persisted user/assistant messages, matching the roles included by the count.
3. Persisted snapshot shape: no normalization change was needed. SessionDB.get_messages_as_conversation() explicitly returns the OpenAI conversation/replay shape consumed by run_conversation; fields such as tool-call data, reasoning sidecars, and api_content preserve replay correctness, while provider conversion handles wire filtering. Stripping them here would reduce replay fidelity rather than harden the path.

While validating the read path, I also found that the production SessionStore.load_transcript() historically converted database errors into [], which made /refine's distinct read-error response unreachable. The new keyword-only strict mode preserves existing best-effort behavior for all other callers but lets /refine fail closed on an unavailable database, compression-tip lookup failure, or message query failure.

Validation:

• scripts/run_tests.sh tests/gateway/test_refine_command.py tests/gateway/test_load_transcript_db_only.py
• 10 passed
• Independent review completed with no blocking findings.

[Enough1122]

AI code review — automated review for reference — not a maintainer.

Thanks for the pass — the lock-held peek_session_id() read-only lookup and the persisted user/assistant messages wording address points 1 and 2. The point-3 explanation is fair: get_messages_as_conversation() deliberately returns the replay shape consumed downstream, so normalizing there would cost fidelity rather than harden the path. Looks consistent.

[fangliquanflq]

Thanks for confirming. The current head keeps the lock-held read-only session lookup, reports the persisted user/assistant count precisely, and preserves the canonical replay shape returned by get_messages_as_conversation(). No further code change is needed for these three points. The latest head (faf5382b3) has all current required checks passing.