fix(web): use latest assistant replies for session recency - #1512
fix(web): use latest assistant replies for session recency#1512techotaku39 wants to merge 23 commits into
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Findings
-
[Major] Legacy reply-clock backfill runs synchronously for every session during hub construction, and sessions with no visible reply remain NULL so the full transcript scan repeats after every restart. Evidence:
hub/src/sync/sessionCache.ts:166is reached fromreloadAll()athub/src/sync/sessionCache.ts:247.
Suggested fix:if (!stored.assistantReplyClockBackfilled) { this.queueAssistantReplyBackfill(sessionId) } // The bounded background worker persists both the nullable timestamp // and assistant_reply_clock_backfilled = true.
-
[Minor] Fork batch hydration now updates the session row once per assistant message and then decodes the entire destination transcript again. Evidence:
hub/src/store/messages.ts:336andhub/src/store/messages.ts:340.
Suggested fix:let latestReply: { content: unknown; createdAt: number } | null = null // During the existing insert loop, retain only the newest assistant reply. if (isAssistantTextMessage(message.content) && (latestReply === null || createdAt > latestReply.createdAt)) { latestReply = { content: message.content, createdAt } } if (latestReply) { touchLastAssistantMessageAt(db, sessionId, latestReply.content, latestReply.createdAt) }
Summary
Review mode: initial
Two performance regressions found in startup backfill and fork hydration. No additional correctness, security, or data-loss findings above the confidence threshold.
Testing
Not run (automation/security policy). GitHub test check passed; git diff --check passed.
HAPI Bot
| && !this.assistantReplyBackfillAttemptedSessionIds.has(sessionId)) { | ||
| this.assistantReplyBackfillAttemptedSessionIds.add(sessionId) | ||
| let latestAssistantMessageAt: number | null = null | ||
| for (const message of this.store.messages.getAllMessages(sessionId)) { |
There was a problem hiding this comment.
[MAJOR] Eager backfill blocks hub startup and repeats for no-reply sessions
This is called by reloadAll(), which invokes refreshSession() for every stored session while SyncEngine is being constructed. After the v24 migration every legacy row is NULL, so startup synchronously loads, decompresses, and parses every message in every transcript before the hub is ready. A session with no visible assistant prose stays NULL, so the same full scan repeats on every process restart.
Persist a separate completion marker and process these scans in bounded background batches:
if (!stored.assistantReplyClockBackfilled) {
this.queueAssistantReplyBackfill(sessionId)
}
// Worker persists last_assistant_message_at (possibly null)
// together with assistant_reply_clock_backfilled = true.| invoked_at: invokedAt ?? null, | ||
| scheduled_at: message.scheduledAt ?? null | ||
| }) | ||
| touchLastAssistantMessageAt(db, sessionId, message.content, createdAt) |
There was a problem hiding this comment.
[MINOR] Keep fork hydration as a true batch operation
This adds one UPDATE sessions for every visible assistant message, then line 340 scans and decodes the entire destination transcript again. Current-session forks can copy an unbounded full history, so the helper no longer has the batch behavior described above it.
Track the newest copied reply during the insert loop and touch the session once; because this path only inserts messages, the final full refresh is unnecessary:
let latestReply: { content: unknown; createdAt: number } | null = null
if (isAssistantTextMessage(message.content)
&& (latestReply === null || createdAt > latestReply.createdAt)) {
latestReply = { content: message.content, createdAt }
}
if (latestReply) {
touchLastAssistantMessageAt(db, sessionId, latestReply.content, latestReply.createdAt)
}
There was a problem hiding this comment.
Findings
-
[Major] Reply-clock recomputation still decodes an unbounded full transcript —
findLastAssistantMessageAtmaterializes and decompresses every row, and it is called from merge and rewind while their write transactions are open (hub/src/store/messages.ts:46, relatedhub/src/store/messages.ts:1014andhub/src/store/messages.ts:1091). The startup worker has the same per-session issue athub/src/sync/sessionCache.ts:296: moving one session to each timer tick prevents an all-session startup stall, but one large/no-reply transcript can still freeze REST/SSE and hold SQLite locks.
Suggested fix:// Scan newest-first in bounded pages. For startup, schedule the next page // on a later tick; for merge/truncate, mark the clock unbackfilled and let // the same worker recompute it outside the write transaction. const rows = getAssistantClockPage(sessionId, cursor, 200) const reply = rows.find(row => isAssistantTextMessage(decodeMessageContent(row.content)) )
-
[Minor] Imported Codex sessions without a visible reply display a different clock from the one used to sort them — the list sorter falls back to
updatedAt, but this branch still rendersimportedAtatweb/src/components/SessionRowSummary.tsx:92. A newly imported no-reply session can therefore appear near the top while showing an old timestamp, preserving the ambiguity this change is intended to remove.
Suggested fix:return formatRelativeTime( session.lastAssistantMessageAt ?? session.updatedAt, t )
Summary
Review mode: follow-up after new commits
The follow-up removes the per-message copy-batch updates, but reply-clock recomputation remains unbounded per transcript. One additional timestamp consistency issue found. No correctness, security, or data-loss findings above the confidence threshold beyond these items.
Testing
Not run (automation/security policy). Suggested coverage: a large legacy no-reply transcript with bounded/yielding backfill, and an imported Codex no-reply session whose displayed timestamp matches its sort fallback.
HAPI Bot
|
|
||
| function findLastAssistantMessageAt(db: Database, sessionId: string): number | null { | ||
| const rows = db.prepare( | ||
| 'SELECT content, created_at FROM messages WHERE session_id = ?' |
There was a problem hiding this comment.
[MAJOR] Bound transcript scans and keep them out of write transactions
This query materializes every message and then decompresses/decodes every payload. The helper is called twice by mergeSessionMessages while BEGIN is open and once by truncateMessagesFromLocalId inside its transaction; the new startup worker also performs the equivalent unbounded getAllMessages scan. One large or no-reply transcript can therefore block the hub event loop and hold the SQLite write lock even though sessions are scheduled one per timer tick.
Use newest-first bounded pages, yielding between pages in the background worker. For merge/truncate, mark the clock unbackfilled and queue the same recomputation after the transaction:
const rows = getAssistantClockPage(sessionId, cursor, 200)
const reply = rows.find(row =>
isAssistantTextMessage(decodeMessageContent(row.content))
)
if (!reply && rows.length === 200) {
scheduleNextPage(sessionId, nextCursor(rows))
}| // Imported Codex sessions used to show the local import time because the | ||
| // list only had `updatedAt`. Once a persisted assistant-reply timestamp is | ||
| // available, prefer it so imported and native sessions share one meaning. | ||
| if (importedAt !== null && session.lastAssistantMessageAt == null) { |
There was a problem hiding this comment.
[MINOR] Keep the displayed fallback clock aligned with sorting
When lastAssistantMessageAt is null, default ordering uses updatedAt, but imported Codex rows still display importedAt here. A no-reply import can sort as newly active while showing an old transcript time, so the right-side timestamp no longer explains its position.
Use the same fallback as the sorter:
return formatRelativeTime(
session.lastAssistantMessageAt ?? session.updatedAt,
t
)
There was a problem hiding this comment.
Findings
- [Major] Authoritative reply-clock recomputations can be discarded by connected web clients — transcript edits intentionally let the backfill write an earlier timestamp or
null, but the completion event is emitted as a structured patch athub/src/sync/sessionCache.ts:347; both detail and list consumers clamp that patch monotonically atweb/src/hooks/useSSE.ts:169andweb/src/hooks/useSSE.ts:627. For a long transcript, themessages-invalidatedrefetch can complete before the paginated scan, after which the authoritative lower/null patch is ignored and the sidebar keeps the deleted reply timestamp and ordering until another full refetch or reload.
Suggested fix:A versioned reply-clock patch that accepts lower/null values only when its version is newer would also preserve stale-event protection.session.lastAssistantMessageAt = refreshed.lastAssistantMessageAt this.publisher.emit({ type: 'session-updated', sessionId, namespace: session.namespace, data: { ...session } })
Questions
- None.
Summary
Review mode: follow-up after new commits
The previous unbounded-scan and imported-time findings are no longer present at the latest head. One Major correctness issue remains in propagation of rewind/merge recomputations. No additional security, data-loss, or performance findings above the confidence threshold.
Testing
Not run (automation/security policy). Suggested coverage: cache a session summary/detail, rewind away the latest reply in a transcript spanning multiple yielded pages, let the invalidation fetch happen before backfill completion, and assert the later SSE update moves the clock to the earlier reply or null and re-sorts the list.
HAPI Bot
| type: 'session-updated', | ||
| sessionId, | ||
| namespace: session.namespace, | ||
| data: { lastAssistantMessageAt: refreshed.lastAssistantMessageAt } satisfies SessionPatch |
There was a problem hiding this comment.
[MAJOR] Propagate authoritative backward recomputations
This backfill result is not always monotonic: rewind/truncation can move the clock to an earlier reply or null. The web patch handlers intentionally apply Math.max and ignore null (web/src/hooks/useSSE.ts:169 and web/src/hooks/useSSE.ts:627), so this structured patch can never correct an already-cached newer value. With a multi-page scan, the invalidation refetch may finish first and leave the stale ordering indefinitely.
Emit an authoritative full-session snapshot for recomputation results, or version the field so newer recomputations may decrease it:
session.lastAssistantMessageAt = refreshed.lastAssistantMessageAt
this.publisher.emit({
type: 'session-updated',
sessionId,
namespace: session.namespace,
data: { ...session }
})
There was a problem hiding this comment.
Findings
- [Major] Newly created Codex imports leave the reply clock stale in the live cache — the new summary field is read from the cached session at
shared/src/sessionSummary.ts:225, but the direct import path creates/caches the session before inserting transcript rows (hub/src/web/routes/codexDesktop.ts:2036,hub/src/web/routes/codexDesktop.ts:2076) and skips message events for newly created sessions (hub/src/web/routes/codexDesktop.ts:2085). The store write updates SQLite only, so the cached value remainsnulland sorting/display falls back to the final activity timestamp until a later full refresh or hub restart.
Suggested fix:if (engine) { engine.recordSessionActivity(sessionId, latestMessageCreatedAt) engine.handleRealtimeEvent({ type: 'session-updated', sessionId }) } else { options.store.sessions.touchSessionUpdatedAt(sessionId, latestMessageCreatedAt, options.namespace) }
Questions
- None.
Summary
Review mode: follow-up after new commits
The prior backward-recomputation propagation issue is no longer present at this head. One Major cache-coherency regression remains for newly created direct Codex imports. No additional security, data-loss, or performance findings met the confidence threshold.
Testing
Not run (automation/security policy). Add a regression that creates a Codex import through the live engine, batch-inserts a transcript ending after its latest assistant prose, and asserts both the engine session and GET /api/sessions expose the assistant reply timestamp rather than the activity fallback.
HAPI Bot
| thinking: session.thinking, | ||
| activeAt: session.activeAt, | ||
| updatedAt: session.updatedAt, | ||
| lastAssistantMessageAt: session.lastAssistantMessageAt ?? null, |
There was a problem hiding this comment.
[MAJOR] Refresh newly created Codex imports after batch insertion
This summary now trusts the cached Session.lastAssistantMessageAt, but the direct Codex import path creates/caches a session before inserting its transcript (hub/src/web/routes/codexDesktop.ts:2036 and hub/src/web/routes/codexDesktop.ts:2076) and skips message events when created is true (hub/src/web/routes/codexDesktop.ts:2085). touchLastAssistantMessageAt updates SQLite only, so the live cache stays null; the new row then displays and sorts by the final activity/tool/user timestamp until a full refresh or restart.
Refresh the session after the batch write:
if (engine) {
engine.recordSessionActivity(sessionId, latestMessageCreatedAt)
engine.handleRealtimeEvent({ type: 'session-updated', sessionId })
} else {
options.store.sessions.touchSessionUpdatedAt(sessionId, latestMessageCreatedAt, options.namespace)
}Please cover the newly-created import path by asserting the engine cache and GET /api/sessions expose the transcript's latest visible assistant timestamp.
There was a problem hiding this comment.
Findings
- [Major] Version backward/null reply-clock replacements — the recomputation emits an unversioned full session record, while the web app processes the same update on independent global and selected-session SSE connections. After rewind/merge, a pre-backfill full record containing the old timestamp can arrive after the corrected full record from the other connection; the full-record branch replaces the cache unconditionally, restoring the deleted timestamp and wrong sidebar order until another refetch. Evidence:
hub/src/sync/sessionCache.ts:351, with unconditional replacement atweb/src/hooks/useSSE.ts:823and dual subscriptions atweb/src/App.tsx:352andweb/src/App.tsx:364.
Suggested fix:Carry the version throughdata: { lastAssistantMessageAt: refreshed.lastAssistantMessageAt, lastAssistantMessageVersion: refreshed.seq }
SessionPatchSchema/SessionSummary, emit it for forward updates too, and apply either a full record or patch only when its reply-clock version is at least the cached version.
Questions
- None.
Summary
Review mode: follow-up after new commits
The prior newly-created Codex import cache finding is fixed at this head. One Major cross-SSE ordering regression remains for reply-clock recomputations. No additional security, data-loss, performance, or maintainability findings met the confidence threshold.
Testing
Not run (automation/security policy). Add a web regression that delivers the corrected backward/null clock before an older pre-backfill full record on the second SSE path and asserts the higher reply-clock version remains authoritative.
HAPI Bot
| // backward or clear it after merge/rewind. Send the complete | ||
| // current record so web clients replace their cached summary | ||
| // instead of applying the monotonic structured-patch path. | ||
| data: { ...session } |
There was a problem hiding this comment.
[MAJOR] Version exact reply-clock replacements
This full record may intentionally lower or clear the clock, but it has no reply-clock version. The app has independent global and selected-session EventSources, and both unconditionally apply full session records. A pre-backfill refresh carrying the old timestamp can therefore arrive after this corrected record on the other connection and restore the stale value.
Suggested fix:
data: {
lastAssistantMessageAt: refreshed.lastAssistantMessageAt,
lastAssistantMessageVersion: refreshed.seq
}Carry that version through the schema/summary, emit it for forward updates, and reject older reply-clock versions for both full records and patches.
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The prior Major finding about stale dual-SSE delivery restoring a pre-rewind/pre-backfill reply clock is addressed by carrying the session sequence through reply-clock patches and summaries, then rejecting older full records and patches in both detail and list caches.
Residual risk: the asynchronous legacy backfill and dual-EventSource ordering paths were reviewed statically only; real large-database startup behavior and browser-level cross-connection timing remain integration-sensitive.
Testing
Not run (automation/security policy). Existing added coverage exercises migration/backfill, rewind recomputation, Codex import cache refresh, default list sorting/time rendering, and stale backward/null SSE ordering.
HAPI Bot
heavygee
added
bug
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The new head merges current main into the previously reviewed implementation; the production integration remains coherent across message ingest/import/copy, merge and rewind recomputation, REST/session-list ordering, and dual-SSE cache version gates.
Residual risks: large-database asynchronous backfill behavior and browser-level cross-connection event timing remain integration-sensitive and were reviewed statically only.
Testing
Not run (automation/security policy). Existing PR coverage targets migration/backfill, rewind recomputation, direct Codex import cache refresh, list sorting/time rendering, and stale backward/null SSE delivery.
HAPI Bot
There was a problem hiding this comment.
Findings
-
[Major] Count visible
event/messagereplies in the reply clock —isAssistantTextMessagefalls through toextractAssistantPlainText, which only recognizescodexandoutputpayloads. HAPI also storessendSessionEvent({ type: 'message' })as an agentevent/message, and the web normalizer renders it as assistant text. Those visible replies therefore never updatelastAssistantMessageAt, leaving affected sessions sorted and timestamped by an older reply or the activity fallback. Evidenceshared/src/messages.ts:156(related context:cli/src/api/apiSession.ts:1053,web/src/chat/normalizeAgent.ts:982).Suggested fix:
if (isObject(record.content) && record.content.type === 'event') { const data = isObject(record.content.data) ? record.content.data : null return data?.type === 'message' && typeof data.message === 'string' && data.message.trim().length > 0 }
Questions
- None.
Summary
Review mode: follow-up after new commits
One Major correctness issue remains in the assistant-reply classifier. No other high-confidence issues found in the latest full diff. Residual risk remains around large-database asynchronous backfill timing and dual-SSE ordering, both reviewed statically only.
Testing
Not run (automation/security policy). GitHub test and integration checks passed. Add a regression test covering a role-wrapped event payload with data.type === 'message' and verify ingest/backfill advances lastAssistantMessageAt.
HAPI Bot
| if (!isClaudeChatVisibleMessage({ type: data.type, subtype: data.subtype })) return false | ||
| } | ||
|
|
||
| return Boolean(extractAssistantPlainText(record.content)?.trim()) |
There was a problem hiding this comment.
[MAJOR] Count visible event/message replies in the reply clock
This falls through to extractAssistantPlainText, which only recognizes codex and output. However, ApiSessionClient.sendSessionEvent({ type: 'message' }) persists { role: 'agent', content: { type: 'event', data: { type: 'message', message } } } (cli/src/api/apiSession.ts:1053), and the web normalizer renders that envelope as assistant text (web/src/chat/normalizeAgent.ts:982). Those established visible replies never advance lastAssistantMessageAt, so the new ordering/time label remains stale for those paths.
Suggested fix:
if (isObject(record.content) && record.content.type === 'event') {
const data = isObject(record.content.data) ? record.content.data : null
return data?.type === 'message'
&& typeof data.message === 'string'
&& data.message.trim().length > 0
}
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No high-confidence issues found in the latest full diff. The prior Major finding for visible event/message replies is addressed by the new classifier path and regression coverage. Residual risk remains around asynchronous backfill behavior on very large, continuously changing legacy transcripts and real browser delivery ordering across dual SSE connections; these were reviewed statically.
Testing
Not run (automation/security policy). GitHub test and integration checks passed; git diff --check passed.
HAPI Bot
There was a problem hiding this comment.
Findings
- [Major] Extend the strict native patch ports for the reply-clock fields — the hub now emits
lastAssistantMessageAtandlastAssistantMessageVersionfor visible assistant replies, but Android omits both from its strict key set (android/core/protocol/src/main/kotlin/app/hapi/protocol/wire/SessionPatch.kt:64) and iOS rejects unknown patch keys (ios/Packages/HapiKit/Sources/HapiProtocol/Models/SessionPatch.swift:114). Both native stores route the resulting unrecognized event to REST fallback (android/core/data/src/main/kotlin/app/hapi/data/store/SessionStore.kt:384,ios/Packages/HapiKit/Sources/HapiClient/Stores/SessionListStore.swift:261), so reply events can cause repeated list/detail refetches across the dual SSE streams and native caches never receive the new version gate. Evidenceshared/src/schemas.ts:381.
Suggested fix:Add the fields to native// Android; mirror with PatchField<Int> in Swift, then port apply/sort gates. val lastAssistantMessageAt: OptionalField<Long?> = OptionalField.Absent val lastAssistantMessageVersion: Long? = null private val KNOWN_KEYS = setOf( // ... "lastAssistantMessageAt", "lastAssistantMessageVersion", )
Session/SessionSummary, apply the same sequence gate, and add a generated SSE fixture for backward/null updates so Android and iOS conformance runs cover this contract change.
Questions
- None.
Summary
Review mode: follow-up after new commits
One Major protocol-conformance issue remains after the merge from main: the TypeScript wire contract changed without updating the strict native ports.
Testing
Not run (automation/security policy). GitHub drift-gate, integration, and test checks passed. Native conformance checks were not present for this head because no generated SSE fixture changed.
HAPI Bot
| activeTurnStartedAt: z.number().nullable().optional(), | ||
| activeAt: z.number().optional(), | ||
| updatedAt: z.number().optional(), | ||
| lastAssistantMessageAt: z.number().nullable().optional(), |
There was a problem hiding this comment.
[MAJOR] Keep the strict native patch ports in sync
These two new keys are emitted by SessionCache.recordAssistantMessage for visible replies, but Android does not include them in SessionPatches.KNOWN_KEYS and iOS rejects them via rejectUnknownKeys. Both native session stores therefore classify each reply-clock patch as unrecognized and fall back to REST list/detail refreshes; with global and session SSE streams, this can duplicate those requests. The native caches also never receive the sequence gate that prevents stale clock restoration.
Suggested fix:
// Android; mirror with PatchField<Int> in Swift, then port apply/sort gates.
val lastAssistantMessageAt: OptionalField<Long?> = OptionalField.Absent
val lastAssistantMessageVersion: Long? = null
private val KNOWN_KEYS = setOf(
// ...
"lastAssistantMessageAt", "lastAssistantMessageVersion",
)Also propagate the fields through native Session/SessionSummary and add a generated SSE fixture covering backward/null versioned updates, so both conformance suites exercise the new contract.
There was a problem hiding this comment.
Findings
-
[Major] Extend the reply-clock sequence gate to REST hydration — the new guards only run for full records received through the SSE handler. A
/sessionsor/sessions/:idrequest can snapshot seq N, then a newer SSE correction at seq N+1 can land before the HTTP response completes; the normal query/native refresh path then writes the older response unconditionally and restores the stale timestamp or null. Evidenceweb/src/hooks/useSSE.ts:82, related writesweb/src/hooks/queries/useSession.ts:30,web/src/hooks/queries/useSessions.ts:12,android/core/data/src/main/kotlin/app/hapi/data/store/SessionStore.kt:168, andios/Packages/HapiKit/Sources/HapiClient/Stores/SessionListStore.swift:119.
Suggested fix:const incoming = await api.getSession(sessionId) const current = queryClient.getQueryData<SessionResponse>(key) return current?.session && incoming.session.seq < current.session.seq ? current : incoming
Apply the same per-row
lastAssistantMessageVersionmerge to list refreshes and mirror the guard in Android/iOS REST writers. -
[Minor] Update the native client contract for the new clock and ordering rules — the schema adds reply-clock fields, while the contract still says full sessions always replace cached state and lists sort by
updatedAt. Native-client implementers following those docs will miss the new stale-record gate. Evidenceshared/src/schemas.ts:381, related stale textdocs/api/client-contract/sse.md:159anddocs/api/client-contract/rest.md:33.
Suggested fix:Full Session: accept only when incoming.seq >= cached.seq. Reply clock: apply lastAssistantMessageAt using lastAssistantMessageVersion. Default list recency: lastAssistantMessageAt ?? updatedAt.
Questions
- None.
Summary
Review mode: follow-up after new commits
One Major cache-ordering hole remains across REST hydration; the client-contract prose also describes obsolete full-record and sorting behavior.
Testing
Not run (automation/security policy). Add deferred-response tests that start a REST fetch at seq N, apply a seq N+1 backward/null reply-clock event, then resolve the fetch and assert the newer detail and summary remain. Current checks: app-build, integration, package-tests, build, and drift-gate passed; test was pending at review time.
HAPI Bot
| * deliver the same event out of order, so an older record must not replace a | ||
| * newer rewind/backfill result. | ||
| */ | ||
| export function shouldAcceptSessionRecord(current: Session | undefined, incoming: Session): boolean { |
There was a problem hiding this comment.
[MAJOR] Apply this sequence gate to REST cache writes too
This protects only full records delivered by the SSE handler. The regular query paths (useSession / useSessions) still let an in-flight HTTP response replace the cache unconditionally; Android loadSessionDetail / refresh and the iOS equivalents do the same. If a request snapshots seq N, a newer backward/null reply-clock correction at seq N+1 can land over SSE, and then the delayed REST response restores the stale clock.
Suggested fix:
const incoming = await api.getSession(sessionId)
const current = queryClient.getQueryData<SessionResponse>(key)
return current?.session && incoming.session.seq < current.session.seq
? current
: incomingUse the summary watermark for /sessions merges and mirror the guard in both native REST writers.
| activeTurnStartedAt: z.number().nullable().optional(), | ||
| activeAt: z.number().optional(), | ||
| updatedAt: z.number().optional(), | ||
| lastAssistantMessageAt: z.number().nullable().optional(), |
There was a problem hiding this comment.
[MINOR] Update the client-contract prose with these new wire rules
docs/api/client-contract/sse.md still says a full Session replaces the cache wholesale, and rest.md still documents default ordering by updatedAt. Those instructions now omit both reply-clock fields and the sequence gate native clients need.
Suggested fix:
Full Session: accept only when incoming.seq >= cached.seq.
Reply clock: apply lastAssistantMessageAt using lastAssistantMessageVersion.
Default list recency: lastAssistantMessageAt ?? updatedAt.
There was a problem hiding this comment.
Findings
-
[Major] Make the Android full-session gate atomic — the global and session SSE pipes run on separate
Dispatchers.Defaultcoroutines, so both handlers can pass the pre-read sequence check and the older record can still write last. Evidenceandroid/core/data/src/main/kotlin/app/hapi/data/store/SessionStore.kt:389.
Suggested fix:_details.update { current -> val cached = current[sessionId] if (cached != null && full.seq < cached.seq) current else current + (sessionId to full) }
-
[Minor] Render the reply clock used by native sorting — Android and iOS now order rows by
lastAssistantMessageAt ?? updatedAt, but both row views still displayupdatedAt; a row can therefore appear above another while showing an older timestamp. Evidenceandroid/core/protocol/src/main/kotlin/app/hapi/protocol/wire/SessionSorting.kt:37andios/Packages/HapiKit/Sources/HapiProtocol/Models/SessionSorting.swift:35; related rendering contextandroid/app/src/main/kotlin/app/hapi/companion/feature/sessions/SessionListScreen.kt:339andios/Hapi/Features/Sessions/SessionListView.swift:250.
Suggested fix:localizedRelativeAge(summary.lastAssistantMessageAt ?: summary.updatedAt)
formatRelativeAge( now: now, thenEpochMs: row.summary.lastAssistantMessageAt ?? row.summary.updatedAt )
Questions
- None.
Summary
Review mode: follow-up after new commits
The prior REST-hydration and client-contract findings are no longer present. Two native-client regressions remain: the Android full-record sequence gate is not atomic under the dual-SSE execution model, and native timestamps no longer explain the new row ordering.
Testing
Not run (automation/security policy). Add a concurrent Android test that delivers seq N+1 and seq N full records through separate scopes and asserts N+1 remains, plus Android/iOS row-label tests for the reply/activity fallback. At review time, Android build, fixture drift gate, iOS package/app builds, and integration passed; the main test job was still running.
HAPI Bot
| val full = parseFullSession(data)?.takeIf { it.id == sessionId } | ||
| if (full != null) { | ||
| _details.update { it + (sessionId to full) } | ||
| val currentDetail = _details.value[sessionId] |
There was a problem hiding this comment.
[MAJOR] Make this sequence check part of the atomic update
The global pipe and the chat session pipe run in separate Dispatchers.Default scopes. Both handlers can read the same old _details.value, both pass this check, and then the seq N record can write after seq N+1. That leaves the detail cache with the stale reply clock despite the new gate.
Suggested fix:
_details.update { current ->
val cached = current[sessionId]
if (cached != null && full.seq < cached.seq) current
else current + (sessionId to full)
}| return@Comparator right.pendingRequestsCount - left.pendingRequestsCount | ||
| } | ||
| right.updatedAt.compareTo(left.updatedAt) | ||
| val leftRecency = left.lastAssistantMessageAt ?: left.updatedAt |
There was a problem hiding this comment.
[MINOR] Update the native row labels to use this same clock
This comparator now orders by reply time, but Android still renders summary.updatedAt (SessionListScreen.kt:339), and iOS has the same mismatch (SessionSorting.swift:35 vs SessionListView.swift:250). Users can see a row above another while its displayed timestamp looks older.
Suggested fix:
localizedRelativeAge(summary.lastAssistantMessageAt ?: summary.updatedAt)formatRelativeAge(
now: now,
thenEpochMs: row.summary.lastAssistantMessageAt ?? row.summary.updatedAt
)
There was a problem hiding this comment.
Findings
- [Major] Exclude status events and sidechain output from the reply clock —
event/messageis the status-event family (for example, “Model changed…” and “Compacting conversation.”), whileoutput/assistantcan carryisSidechain: truesubagent progress. The new classifier accepts both, so these records durably advancelastAssistantMessageAtand reorder every client as though the main agent had replied. Evidenceshared/src/messages.ts:106andshared/src/messages.ts:154; related message-shape contextweb/scripts/fixtures/cases/events.ts:93andweb/scripts/fixtures/cases/sidechain.ts:98.
Suggested fix:const messageContent = record.content if (!isObject(messageContent) || messageContent.type === 'event') return false if (messageContent.type === 'output') { const data = isObject(messageContent.data) ? messageContent.data : null if (!data || data.isSidechain === true || Boolean(data.isMeta) || Boolean(data.isCompactSummary)) return false }
Questions
- None.
Summary
Review mode: follow-up after new commits
The two prior native-client findings are no longer present. One reply-clock classification defect remains: status/subagent activity can be persisted and displayed as the latest assistant reply.
Testing
Not run (automation/security policy). Add classifier and storage/backfill tests proving event/message status rows and output/assistant rows with isSidechain: true do not change lastAssistantMessageAt.
HAPI Bot
| if (!isObject(content)) return null | ||
|
|
||
| if (content.type === 'codex') { | ||
| if (content.type === 'codex' || content.type === 'event') { |
There was a problem hiding this comment.
[MAJOR] Keep status/subagent activity out of the reply clock
event/message is the status-event family, not an assistant reply—the existing fixtures use it for “Model changed…” and “Compacting conversation.” (web/scripts/fixtures/cases/events.ts:93). The same classifier also accepts output/assistant text with isSidechain: true, which is nested subagent progress (web/scripts/fixtures/cases/sidechain.ts:98). Because message ingest and legacy backfill both call this helper, either shape durably advances lastAssistantMessageAt and reorders every client as if the main agent replied.
Suggested fix:
const messageContent = record.content
if (!isObject(messageContent) || messageContent.type === 'event') return false
if (messageContent.type === 'output') {
const data = isObject(messageContent.data) ? messageContent.data : null
if (!data || data.isSidechain === true || Boolean(data.isMeta) || Boolean(data.isCompactSummary)) return false
}
There was a problem hiding this comment.
Findings
- [Major] Re-fetch after rejecting a stale REST snapshot — the new gates return the entire cached detail/summary when a delayed response has an older reply-clock sequence. That cache may contain only the newer reply patch and still be missing unrelated changes captured by the discarded response. Example: cache at seq 8 / metadata v1; REST captures seq 9 / metadata v2; reply patch seq 10 lands; the seq-9 response is discarded, so metadata v2 remains absent on a quiet session. The same behavior exists in Android and iOS. Evidence
web/src/lib/sessionCache.ts:59,web/src/lib/sessionCache.ts:78,android/core/data/src/main/kotlin/app/hapi/data/store/SessionStore.kt:173,android/core/data/src/main/kotlin/app/hapi/data/store/SessionStore.kt:207,ios/Packages/HapiKit/Sources/HapiClient/Stores/SessionListStore.swift:123, andios/Packages/HapiKit/Sources/HapiClient/Stores/SessionListStore.swift:162.
Suggested fix:Apply the same bounded retry to list refreshes and the native stores.const first = await api.getSession(sessionId) const current = queryClient.getQueryData<SessionResponse>(queryKeys.session(sessionId)) if (current?.session && first.session.seq < current.session.seq) { // One bounded recovery fetch after the newer SSE watermark. const fresh = await api.getSession(sessionId) return mergeSessionResponse(current, fresh) } return first
Questions
- None.
Summary
Review mode: follow-up after new commits
The prior reply-classification finding is addressed. One cross-client cache-recovery defect remains: a rejected delayed REST response is treated as a successful refresh, allowing unrelated intermediate fields to stay stale indefinitely on a quiet session.
Testing
Not run (automation/security policy). Add Web, Android, and iOS race tests with metadata v2 at seq 9, a reply-clock patch at seq 10, and a delayed seq-9 REST response; verify a bounded follow-up fetch preserves the seq-10 clock and loads metadata v2.
HAPI Bot
| incoming: SessionResponse | ||
| ): SessionResponse { | ||
| return current?.session && !shouldAcceptSessionRecord(current.session, incoming.session) | ||
| ? current |
There was a problem hiding this comment.
[MAJOR] Re-fetch after rejecting a stale REST snapshot
Returning the entire cached record protects the newer reply clock, but it also treats this REST request as successfully hydrated. The cache may only have received the seq-10 reply patch and still be missing an unrelated seq-9 update contained in this response (for example metadata/title, pin state, or pending-request fields). In that race, the discarded response is the only copy of the intermediate update and the quiet session stays stale. mergeSessionsResponse has the same problem, as do the Android/iOS ports.
Suggested fix:
const first = await api.getSession(sessionId)
const current = queryClient.getQueryData<SessionResponse>(queryKeys.session(sessionId))
if (current?.session && first.session.seq < current.session.seq) {
const fresh = await api.getSession(sessionId) // one bounded recovery fetch
return mergeSessionResponse(current, fresh)
}
return firstApply the same bounded retry to list refreshes and native stores; add a race test where metadata v2/seq 9 precedes a reply patch at seq 10 and the delayed seq-9 response arrives last.
There was a problem hiding this comment.
Findings
- [Major] Exclude hidden metadata/tool-only records from the reply clock —
isAssistantTextMessageaccepts any extracted non-empty text, but the chat hides a footer-onlyAGENT_NOTIFY_SUMMARYby default (web/src/components/AssistantChat/messages/NotifySummaryText.tsx:130) and rendersInside the task-NNN log…AGY messages as tool-call chips (web/src/chat/normalizeAgent.ts:785). Both therefore reorder/display a session as having a new assistant reply when no assistant prose is visible. Evidenceshared/src/messages.ts:170.
Suggested fix:const text = extractAssistantPlainText(record.content) if (!text) return false const visibleText = stripNotifySummaryFooter(text).trim() if (!visibleText) return false const outputData = isObject(record.content) && record.content.type === 'output' && isObject(record.content.data) ? record.content.data : null if (outputData?.type === 'agy_message' && /^Inside the task-\d+ log\b/.test(visibleText)) { return false } return true
Questions
- None.
Summary
Review mode: follow-up after new commits
The prior stale REST-snapshot finding is addressed by bounded retries across Web, Android, and iOS. One reply-classification defect remains: hidden metadata-only and tool-only AGY records can advance the new recency clock.
Testing
Not run (automation/security policy). Add isAssistantTextMessage regressions for a footer-only AGENT_NOTIFY_SUMMARY, prose plus a footer, and Inside the task-266 log...; only the prose-plus-footer case should advance the clock.
HAPI Bot
| if (!isClaudeChatVisibleMessage({ type: data.type, subtype: data.subtype })) return false | ||
| } | ||
|
|
||
| return Boolean(extractAssistantPlainText(record.content)?.trim()) |
There was a problem hiding this comment.
[MAJOR] Exclude hidden metadata/tool-only records from the reply clock
This returns true for two records that the UI does not render as assistant prose:
- a footer-only
AGENT_NOTIFY_SUMMARY, whichNotifySummaryTextstrips tonullby default; - AGY
Inside the task-NNN log…narration, whichnormalizeAgentconverts to anAgyTaskLogtool-call chip.
Both will update lastAssistantMessageAt and reorder the session even though the latest visible content is metadata/tool activity, contradicting the new clock's contract.
Suggested fix:
const text = extractAssistantPlainText(record.content)
if (!text) return false
const visibleText = stripNotifySummaryFooter(text).trim()
if (!visibleText) return false
const outputData = isObject(record.content)
&& record.content.type === 'output'
&& isObject(record.content.data)
? record.content.data
: null
if (outputData?.type === 'agy_message' && /^Inside the task-\d+ log\b/.test(visibleText)) {
return false
}
return trueAdd classifier tests for footer-only, prose-plus-footer, and Inside the task-266 log... inputs.
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The new commit addresses the prior HAPI Bot Major finding by excluding footer-only AGENT_NOTIFY_SUMMARY records and AGY task-log narration from the assistant reply clock, with classifier and storage regressions added. Residual risk is limited to platform-specific compilation and runtime timing paths not exercised in this review.
Testing
Not run (automation/security policy); static review only.
HAPI Bot
…nt-reply-time # Conflicts: # hub/src/store/index.ts # hub/src/store/migration-v13.test.ts # hub/src/store/migration-v15.test.ts # hub/src/store/migration-v18.test.ts # hub/src/store/migration-v19.test.ts # hub/src/store/migration-v20.test.ts # hub/src/store/migration-v23.test.ts # hub/src/store/migration-v24.test.ts # shared/src/messages.test.ts
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No issues found in the latest full diff. The new head only merges current main; the reply-clock schema migration, assistant-message classification, asynchronous backfill, SSE/REST sequence gates, sorting, and native-client parity remain internally consistent after conflict resolution. Residual risk is limited to platform-specific compilation and long-running backfill timing paths not exercised in this static review.
Testing
Not run (automation/security policy); static review only.
HAPI Bot
…nt-reply-time # Conflicts: # shared/src/sessionSummary.ts
There was a problem hiding this comment.
Findings
-
[Major] Shared assistant-text extraction now treats status events as replies —
extractAssistantPlainTextis used by notification fallback and title generation, so a trailingevent/messagesuch as “Model changed…” or “Compacting conversation…” can replace the actual assistant response in notifications and enter title prompts as assistant prose. Evidence:shared/src/messages.ts:150; related consumershub/src/notifications/nativeNotificationComposer.ts:240,hub/src/sync/titleSuggestion.ts:147.Suggested fix:
if (content.type === 'codex') { const data = isObject(content.data) ? content.data : null if (!data || data.type !== 'message') return null return typeof data.message === 'string' && data.message.trim().length > 0 ? data.message : null }
Questions
- None.
Summary
Review mode: follow-up after new commits
One major regression found: the reply-clock work broadens a shared assistant-text helper in a way that changes notifications and title generation outside the intended sorting behavior. The reply-clock persistence, bounded backfill, sequence-gated backward/null corrections, REST/SSE ordering, and native-client parity were otherwise reviewed statically.
Testing
Not run (automation/security policy); static review only. Add a regression test proving an event/message status after a real assistant reply does not become the notification/title assistant text.
HAPI Bot
| if (!isObject(content)) return null | ||
|
|
||
| if (content.type === 'codex') { | ||
| if (content.type === 'codex' || content.type === 'event') { |
There was a problem hiding this comment.
[MAJOR] Keep status events out of the shared assistant-text extractor
extractAssistantPlainText is also used by notification fallback and title generation, not only by the new reply-clock classifier. Accepting event/message here means a trailing status event such as “Model changed…” or “Compacting conversation…” can replace the real assistant response in notifications and be fed to title generation as assistant prose. isAssistantTextMessage already rejects event payloads below, so the reply-clock path does not need this widening.
Suggested fix:
if (content.type === 'codex') {
const data = isObject(content.data) ? content.data : null
if (!data || data.type !== 'message') return null
return typeof data.message === 'string' && data.message.trim().length > 0
? data.message
: null
}If another UI needs event text, add a separate event-specific extractor rather than changing the assistant-text helper’s semantics.
There was a problem hiding this comment.
Findings
- None.
Questions
- None.
Summary
Review mode: follow-up after new commits
No high-confidence issues found in the latest full diff. The prior Major at shared/src/messages.ts:142 is resolved: event/message status text is no longer accepted by shared assistant-text extraction. Residual risk: the asynchronous reply-clock backfill/retry path has unit coverage for paging, truncation, null results, and version gating, but no sustained-concurrency stress test was found.
Testing
Not run locally (automation/security policy). Current GitHub checks for Test, integration, Android build, iOS package/app, and fixture drift gate passed.
HAPI Bot
2 participants
Summary
Use the latest visible assistant reply as the default session-list recency clock while keeping
updatedAtas the activity, unread, and replay clock. This follow-up also makes legacy reply-clock migration asynchronous and bounded, removes redundant fork transcript rescans, and keeps reply-clock updates ordered across dual SSE connections.Problem / Motivation
The sidebar previously used
updatedAtfor both activity and displayed recency. Prompt or activity updates could make a session look newer than another session with a more recent AI response, making the right-side timestamp and default ordering difficult to interpret. Legacy transcript backfills also needed to avoid materializing large histories while holding SQLite write locks. Finally, global and session-scoped SSE connections can deliver full session records out of order while a merge or rewind recomputes the reply clock.Implementation
last_assistant_message_atin schema v24 and the persistedassistant_reply_clock_backfilledmarker in schema v25.updatedAtwhen no visible reply exists.order=updatedAtbehavior unchanged.User Impact
Migration / Rollback
updatedAt.Validation
bun test hub/src/store hub/src/sync— 538 passed, 0 failed.bun test hub/src/web/routes/codexDesktop.test.ts— 30 passed, 0 failed; covers live-cache refresh after a newly created direct Codex import.bunx vitest run src/components/SessionList.test.ts src/components/SessionRowSummary.time.test.ts src/hooks/useSSE.test.ts(fromweb) — 91 passed, 0 failed; includes dual-SSE reply-clock ordering and backward/null update coverage.bun run typecheck:hub— passed.bun run build:hub— passed./api/sessionsreturned 200; seeded reply-recency data exposed the newer reply before the newer activity-only session.git diff --check— passed.Related Issues
Fixes #477
AI Disclosure
Tool: OpenAI Codex.
Model: GPT-5.6.
The implementation, diff review, and validation were performed in the HAPI task worktree.