mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-31 19:16:29 +00:00
fix(desktop): preserve correction order in session tabs
This commit is contained in:
parent
fc39c7ac31
commit
76e9ac3915
4 changed files with 142 additions and 123 deletions
|
|
@ -21,11 +21,9 @@ const INFERENCE_SWITCH_TRIGGER = 'E2E_INFERENCE_SWITCH_TRIGGER'
|
|||
const INFERENCE_PROMPT = `${INFERENCE_SWITCH_TRIGGER}: original inference prompt must remain singular.`
|
||||
const INFERENCE_CORRECTION = `${INFERENCE_SWITCH_TRIGGER}: correction sent while inference is live.`
|
||||
|
||||
// A ⌘T-style tab can put several chat surfaces on the page at once, so every
|
||||
// locator here is scoped to the ACTIVE one (the most recently mounted surface)
|
||||
// rather than `.first()` / a bare document query, which would silently target
|
||||
// the wrong session's composer or transcript.
|
||||
const SURFACE = '[data-composer-target]'
|
||||
// Inactive tabs stay mounted under a data-pane-hidden ancestor. Match the
|
||||
// renderer's keep-alive visibility policy instead of relying on DOM order.
|
||||
const SURFACE = '[data-composer-target]:not([data-pane-hidden] [data-composer-target])'
|
||||
|
||||
function activeSurface(page: Page) {
|
||||
return page.locator(SURFACE).last()
|
||||
|
|
@ -102,7 +100,9 @@ async function transcriptMessageOrder(page: Page): Promise<string[]> {
|
|||
const viewport = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]')
|
||||
if (!viewport) return []
|
||||
|
||||
return Array.from(viewport.querySelectorAll<HTMLElement>('[data-role="user"], [data-role="assistant"]'))
|
||||
return Array.from(
|
||||
viewport.querySelectorAll<HTMLElement>('[data-role="user"], [data-role="assistant"], [data-role="system"]'),
|
||||
)
|
||||
.map(message => message.textContent?.trim() ?? '')
|
||||
.filter(Boolean)
|
||||
}, SURFACE)
|
||||
|
|
@ -150,7 +150,12 @@ async function reopenInferenceSession(page: Page): Promise<void> {
|
|||
}
|
||||
|
||||
function relevantOrder(messages: string[]): string[] {
|
||||
return messages.filter(message => message.includes(ORIGINAL_PROMPT) || message.includes(CORRECTION))
|
||||
return messages.flatMap(message => {
|
||||
if (message.includes(ORIGINAL_PROMPT)) return [ORIGINAL_PROMPT]
|
||||
if (message.includes(CORRECTION)) return [CORRECTION]
|
||||
|
||||
return []
|
||||
})
|
||||
}
|
||||
|
||||
function steerTurnOrder(messages: string[]): string[] {
|
||||
|
|
|
|||
|
|
@ -93,10 +93,9 @@ function sessionRow(page: Page) {
|
|||
return page.locator('[data-slot="sidebar"] button').filter({ hasText: CAPTION }).first()
|
||||
}
|
||||
|
||||
// A ⌘T-style tab can put several chat surfaces on the page at once, so
|
||||
// transcript queries target the ACTIVE (most recently mounted) surface rather
|
||||
// than the first match, which would read the wrong session.
|
||||
const SURFACE = '[data-composer-target]'
|
||||
// Inactive tabs stay mounted under a data-pane-hidden ancestor. Match the
|
||||
// renderer's keep-alive visibility policy instead of relying on DOM order.
|
||||
const SURFACE = '[data-composer-target]:not([data-pane-hidden] [data-composer-target])'
|
||||
|
||||
function activeViewportText(surfaceSelector: string): string {
|
||||
const surfaces = document.querySelectorAll(surfaceSelector)
|
||||
|
|
|
|||
|
|
@ -24,6 +24,8 @@
|
|||
* MutationObserver burst), but `$messages` was still set twice.
|
||||
*
|
||||
* The test passes when bursts === 1 AND reconciles === 0.
|
||||
* The sidebar "+" keeps the session warm in another tab. Its reactivation
|
||||
* follows the same contract: one additive paint and zero reconciles.
|
||||
*
|
||||
* Prerequisite: `npm run build` must have been run so dist/ exists.
|
||||
*/
|
||||
|
|
@ -43,6 +45,10 @@ import { startMockServer } from './mock-server'
|
|||
import { RealSessionBuilder } from './real-session-builder'
|
||||
|
||||
const SESSION_TITLE = 'E2E Warm Resume Jitter Test'
|
||||
|
||||
// Inactive tabs stay mounted under a data-pane-hidden ancestor. Match the
|
||||
// renderer's keep-alive visibility policy instead of relying on DOM order.
|
||||
const SURFACE = '[data-composer-target]:not([data-pane-hidden] [data-composer-target])'
|
||||
/** 32 messages (16 user/assistant pairs) — enough DOM churn for detection. */
|
||||
const MESSAGE_COUNT = 32
|
||||
/** Seeded PRNG so the generated content is deterministic across runs. */
|
||||
|
|
@ -155,8 +161,9 @@ test.afterAll(async () => {
|
|||
* add/remove nodes.
|
||||
*/
|
||||
async function installRenderCounter(page: import('@playwright/test').Page): Promise<void> {
|
||||
await page.evaluate(() => {
|
||||
const viewport = document.querySelector('[data-slot="aui_thread-viewport"]')
|
||||
await page.evaluate((surfaceSelector: string) => {
|
||||
const surfaces = document.querySelectorAll(surfaceSelector)
|
||||
const viewport = surfaces[surfaces.length - 1]?.querySelector('[data-slot="aui_thread-viewport"]')
|
||||
if (!viewport) {
|
||||
throw new Error('Thread viewport not found before warm resume')
|
||||
}
|
||||
|
|
@ -224,7 +231,53 @@ async function installRenderCounter(page: import('@playwright/test').Page): Prom
|
|||
hasMessages = true
|
||||
}
|
||||
}, 2)
|
||||
})
|
||||
}, SURFACE)
|
||||
}
|
||||
|
||||
/** Wait until the ACTIVE chat surface's transcript contains `text`. */
|
||||
async function waitForActiveTranscriptText(
|
||||
page: import('@playwright/test').Page,
|
||||
text: string,
|
||||
timeout = 30_000,
|
||||
): Promise<void> {
|
||||
await page.waitForFunction(
|
||||
([expected, surfaceSelector]: [string, string]) => {
|
||||
const surfaces = document.querySelectorAll(surfaceSelector)
|
||||
const active = surfaces[surfaces.length - 1]
|
||||
|
||||
return (active?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected)
|
||||
},
|
||||
[text, SURFACE] as [string, string],
|
||||
{ timeout },
|
||||
)
|
||||
}
|
||||
|
||||
async function waitForActiveTranscriptWithoutText(
|
||||
page: import('@playwright/test').Page,
|
||||
text: string,
|
||||
): Promise<void> {
|
||||
await page.waitForFunction(
|
||||
([expected, surfaceSelector]: [string, string]) => {
|
||||
const surfaces = document.querySelectorAll(surfaceSelector)
|
||||
const active = surfaces[surfaces.length - 1]
|
||||
|
||||
return !(active?.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(expected)
|
||||
},
|
||||
[text, SURFACE] as [string, string],
|
||||
{ timeout: 15_000 },
|
||||
)
|
||||
}
|
||||
|
||||
/** Replace the primary surface with a draft while retaining its warm cache. */
|
||||
async function openFreshDraft(page: import('@playwright/test').Page, priorText: string): Promise<void> {
|
||||
await page.keyboard.press(process.platform === 'darwin' ? 'Meta+N' : 'Control+N')
|
||||
await waitForActiveTranscriptWithoutText(page, priorText)
|
||||
}
|
||||
|
||||
/** Stack an empty tab while leaving the current transcript mounted and warm. */
|
||||
async function openNewSessionTab(page: import('@playwright/test').Page, priorText: string): Promise<void> {
|
||||
await page.locator('[data-slot="sidebar"] button[aria-label="New session"]').first().click()
|
||||
await waitForActiveTranscriptWithoutText(page, priorText)
|
||||
}
|
||||
|
||||
/** Stop the render counter and return the recorded burst/reconcile counts. */
|
||||
|
|
@ -261,7 +314,7 @@ function assertNoJitter(result: { bursts: number; mutations: number; timeline: n
|
|||
).toBe(0)
|
||||
}
|
||||
|
||||
test('warm-route resume paints transcript exactly once (no jitter)', async ({}, testInfo) => {
|
||||
test('tab reactivation paints the transcript exactly once (no jitter)', async ({}, testInfo) => {
|
||||
const page = fixture!.page
|
||||
|
||||
// Wait for the sidebar to populate with our seeded session.
|
||||
|
|
@ -277,61 +330,22 @@ test('warm-route resume paints transcript exactly once (no jitter)', async ({},
|
|||
|
||||
// Wait for the transcript to appear — the first user message text confirms
|
||||
// the cold-path prefetch painted.
|
||||
await page.waitForFunction(
|
||||
(text: string) =>
|
||||
document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(text) ??
|
||||
false,
|
||||
FIRST_USER_MSG,
|
||||
{ timeout: 30_000 },
|
||||
)
|
||||
await waitForActiveTranscriptText(page, FIRST_USER_MSG)
|
||||
|
||||
// Wait for the session to fully settle (cold-path RPC + reconciliation).
|
||||
await page.waitForTimeout(2_000)
|
||||
|
||||
// Step 2: Navigate away to a new chat — this does NOT evict the warm cache.
|
||||
const newSessionButton = page
|
||||
.locator('[data-slot="sidebar"] button[aria-label="New session"]')
|
||||
.first()
|
||||
await newSessionButton.click()
|
||||
|
||||
// Wait for the new-chat empty state. The "+" opens a NEW TAB beside the
|
||||
// resumed session rather than replacing it, so the old transcript stays
|
||||
// mounted — assert the newly-added surface is the empty one.
|
||||
await page.waitForFunction(
|
||||
(firstMsg: string) => {
|
||||
const surfaces = document.querySelectorAll('[data-composer-target]')
|
||||
const active = surfaces[surfaces.length - 1]
|
||||
if (!active) return false
|
||||
const text = active.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? ''
|
||||
return !text.includes(firstMsg)
|
||||
},
|
||||
FIRST_USER_MSG,
|
||||
{ timeout: 15_000 },
|
||||
)
|
||||
// Observe the loaded transcript before "+" hides it. The old test attached
|
||||
// after the switch and watched the empty draft instead of this session.
|
||||
await installRenderCounter(page)
|
||||
await openNewSessionTab(page, FIRST_USER_MSG)
|
||||
|
||||
await page.waitForTimeout(500)
|
||||
|
||||
// Step 3: Install render counter, click back (warm resume), wait, assert.
|
||||
await installRenderCounter(page)
|
||||
// Step 3: Click back, settle, and assert one paint with no second reconcile.
|
||||
await sessionRow.click()
|
||||
|
||||
await page.waitForFunction(
|
||||
(text: string) =>
|
||||
document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(text) ??
|
||||
false,
|
||||
FIRST_USER_MSG,
|
||||
{ timeout: 30_000 },
|
||||
)
|
||||
|
||||
// Wait for at least 1 burst, then settle.
|
||||
await page.waitForFunction(
|
||||
() => {
|
||||
const w = window as unknown as { __RENDER_COUNT__?: { bursts: number } }
|
||||
return Boolean(w.__RENDER_COUNT__ && w.__RENDER_COUNT__.bursts > 0)
|
||||
},
|
||||
undefined,
|
||||
{ timeout: 10_000 },
|
||||
)
|
||||
await waitForActiveTranscriptText(page, FIRST_USER_MSG)
|
||||
await page.waitForTimeout(2_000)
|
||||
|
||||
const result = await readRenderCount(page)
|
||||
|
|
@ -357,13 +371,7 @@ test('warm-route resume after background inference completes (no jitter)', async
|
|||
|
||||
// Step 1: Cold resume — populate the warm cache.
|
||||
await sessionRow.click()
|
||||
await page.waitForFunction(
|
||||
(text: string) =>
|
||||
document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(text) ??
|
||||
false,
|
||||
FIRST_USER_MSG,
|
||||
{ timeout: 30_000 },
|
||||
)
|
||||
await waitForActiveTranscriptText(page, FIRST_USER_MSG)
|
||||
await page.waitForTimeout(2_000)
|
||||
|
||||
// Step 2: Send a message — triggers inference via the mock server.
|
||||
|
|
@ -376,37 +384,15 @@ test('warm-route resume after background inference completes (no jitter)', async
|
|||
// Wait for the mock response to appear in the transcript, confirming
|
||||
// the turn completed and message.complete fired (which updates the warm
|
||||
// cache via updateSessionState).
|
||||
await page.waitForFunction(
|
||||
() => {
|
||||
const viewport = document.querySelector('[data-slot="aui_thread-viewport"]')
|
||||
return viewport?.textContent?.includes('mock inference server') ?? false
|
||||
},
|
||||
undefined,
|
||||
{ timeout: 60_000 },
|
||||
)
|
||||
await waitForActiveTranscriptText(page, 'mock inference server', 60_000)
|
||||
// Extra settle for message.complete → updateSessionState → cache write.
|
||||
await page.waitForTimeout(2_000)
|
||||
|
||||
// Verify the prompt was received by the mock server.
|
||||
expect(mock.receivedPrompts).toContain(PROMPT)
|
||||
|
||||
// Step 3: Navigate away — the warm cache retains the updated messages.
|
||||
const newSessionButton = page
|
||||
.locator('[data-slot="sidebar"] button[aria-label="New session"]')
|
||||
.first()
|
||||
await newSessionButton.click()
|
||||
// "+" stacks a new tab, so the prior transcript stays mounted in its own
|
||||
// surface — check the newly-added surface rather than the whole page.
|
||||
await page.waitForFunction(
|
||||
(prompt: string) => {
|
||||
const surfaces = document.querySelectorAll('[data-composer-target]')
|
||||
const active = surfaces[surfaces.length - 1]
|
||||
if (!active) return false
|
||||
return !(active.querySelector('[data-slot="aui_thread-viewport"]')?.textContent ?? '').includes(prompt)
|
||||
},
|
||||
PROMPT,
|
||||
{ timeout: 15_000 },
|
||||
)
|
||||
// Step 3: Replace the primary chat; the warm cache retains the updated messages.
|
||||
await openFreshDraft(page, PROMPT)
|
||||
await page.waitForTimeout(500)
|
||||
|
||||
// Step 4: Install render counter, click back (warm resume), wait, assert.
|
||||
|
|
@ -415,13 +401,7 @@ test('warm-route resume after background inference completes (no jitter)', async
|
|||
|
||||
// Wait for the transcript to reappear — the warm cache should already
|
||||
// have the completed turn (updated by message.complete events).
|
||||
await page.waitForFunction(
|
||||
(text: string) =>
|
||||
document.querySelector('[data-slot="aui_thread-viewport"]')?.textContent?.includes(text) ??
|
||||
false,
|
||||
FIRST_USER_MSG,
|
||||
{ timeout: 30_000 },
|
||||
)
|
||||
await waitForActiveTranscriptText(page, FIRST_USER_MSG)
|
||||
|
||||
// Wait for at least 1 burst, then settle.
|
||||
await page.waitForFunction(
|
||||
|
|
|
|||
|
|
@ -236,23 +236,6 @@ export function useSessionTileActions({ runtimeId, scope, storedSessionId }: Ses
|
|||
[listTileSession, scope.attachments.$attachments, submitPromptText]
|
||||
)
|
||||
|
||||
const appendSystemNote = useCallback(
|
||||
(text: string) => {
|
||||
update(state => ({
|
||||
...state,
|
||||
messages: [
|
||||
...state.messages,
|
||||
{
|
||||
id: `system-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`,
|
||||
role: 'system',
|
||||
parts: [textPart(text)]
|
||||
}
|
||||
]
|
||||
}))
|
||||
},
|
||||
[update]
|
||||
)
|
||||
|
||||
const cancelRun = useCallback(async () => {
|
||||
const sessionId = runtimeIdRef.current
|
||||
|
||||
|
|
@ -283,30 +266,82 @@ export function useSessionTileActions({ runtimeId, scope, storedSessionId }: Ses
|
|||
const steerPrompt = useCallback(
|
||||
async (rawText: string): Promise<boolean> => {
|
||||
const text = rawText.trim()
|
||||
const sessionId = runtimeIdRef.current
|
||||
|
||||
if (!text) {
|
||||
if (!text || !sessionId) {
|
||||
return false
|
||||
}
|
||||
|
||||
const messageId = `user-${Date.now()}-${Math.random().toString(36).slice(2, 8)}`
|
||||
const mutate = (updater: (state: ClientSessionState) => ClientSessionState) =>
|
||||
sessionTileDelegate()?.updateSession(sessionId, updater)
|
||||
|
||||
// Match the primary composer: insert the correction before the active
|
||||
// reply before awaiting the redirect RPC, whose completion can race us.
|
||||
mutate(state => {
|
||||
const message = {
|
||||
id: messageId,
|
||||
role: 'user' as const,
|
||||
parts: [textPart(text)]
|
||||
}
|
||||
const streamIndex = state.streamId
|
||||
? state.messages.findIndex(candidate => candidate.id === state.streamId)
|
||||
: -1
|
||||
const lastAssistantIndex = state.messages.map(candidate => candidate.role).lastIndexOf('assistant')
|
||||
const insertionIndex = streamIndex >= 0 ? streamIndex : lastAssistantIndex
|
||||
const messages =
|
||||
insertionIndex >= 0
|
||||
? [...state.messages.slice(0, insertionIndex), message, ...state.messages.slice(insertionIndex)]
|
||||
: [...state.messages, message]
|
||||
|
||||
return { ...state, messages }
|
||||
})
|
||||
|
||||
const discardOptimisticMessage = () =>
|
||||
mutate(state => ({
|
||||
...state,
|
||||
messages: state.messages.filter(message => message.id !== messageId)
|
||||
}))
|
||||
|
||||
const moveOptimisticMessageToEnd = () =>
|
||||
mutate(state => {
|
||||
const message = state.messages.find(candidate => candidate.id === messageId)
|
||||
|
||||
return message
|
||||
? { ...state, messages: [...state.messages.filter(candidate => candidate.id !== messageId), message] }
|
||||
: state
|
||||
})
|
||||
|
||||
try {
|
||||
const result = await requestGateway<{ status?: string }>('session.steer', {
|
||||
session_id: runtimeIdRef.current,
|
||||
const result = await requestGateway<{ status?: string }>('session.redirect', {
|
||||
session_id: sessionId,
|
||||
text
|
||||
})
|
||||
|
||||
if (result?.status === 'queued') {
|
||||
if (result?.status === 'redirected') {
|
||||
triggerHaptic('submit')
|
||||
|
||||
return true
|
||||
}
|
||||
|
||||
if (result?.status === 'queued') {
|
||||
moveOptimisticMessageToEnd()
|
||||
triggerHaptic('submit')
|
||||
appendSystemNote(`steer:${text}`)
|
||||
|
||||
return true
|
||||
}
|
||||
} catch {
|
||||
discardOptimisticMessage()
|
||||
// Swallow — the caller queues the text so nothing is lost.
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
discardOptimisticMessage()
|
||||
|
||||
return false
|
||||
},
|
||||
[appendSystemNote, requestGateway]
|
||||
[requestGateway]
|
||||
)
|
||||
|
||||
// Rewind primitive (interrupt-first for live turns, busy-retry) — shared with
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue