From 69a879b2ead6382b88c1fe325774a8bb51494628 Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Sun, 9 Aug 2026 15:26:55 +0700 Subject: [PATCH 01/12] Resolve reopened Remote UI feedback --- .gitignore | 2 + bin/remote-ui-smoke | 443 +++++++++++++- .../reports/usage-2026-07-06-to-2026-08-06.md | 57 -- .../usage-2026-07-06-to-2026-08-06.query.json | 28 - lib/hq/remote_server.rb | 24 + lib/hq/remote_ui/assets/app.css | 138 +++++ lib/hq/remote_ui/assets/app.js | 545 ++++++++++++++++-- test/remote_server_test.rb | 72 ++- 8 files changed, 1164 insertions(+), 145 deletions(-) delete mode 100644 docs/reports/usage-2026-07-06-to-2026-08-06.md delete mode 100644 docs/reports/usage-2026-07-06-to-2026-08-06.query.json diff --git a/.gitignore b/.gitignore index a410dc8..1b49aef 100644 --- a/.gitignore +++ b/.gitignore @@ -22,3 +22,5 @@ artifacts/design-system/**/*.jpg artifacts/design-system/**/*.jpeg artifacts/design-system/**/*.gif artifacts/design-system/**/*.webp +docs/reports/usage-*.md +docs/reports/usage-*.query.json diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index fd90989..ec964c5 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -301,6 +301,26 @@ def associate_agent_schedule(logs_dir, agent_key) File.write(agents_path, JSON.pretty_generate(agents)) end +def associate_agent_pull_request(logs_dir, agent_key) + agents_path = File.join(logs_dir, "managed_agents.json") + agents = JSON.parse(File.read(agents_path)) + agent = agents.find { |item| item["key"] == agent_key } + raise "Failed to find smoke agent #{agent_key.inspect}" unless agent + + attachments_path = agent.fetch("log_path").sub(/\.raw\.log\z/, ".attachments.json") + File.write( + attachments_path, + JSON.pretty_generate( + "attachments" => [{ + "kind" => "pull_request", + "title" => "Example PR", + "url" => "https://github.com/example/web/pull/123", + "created_at" => Time.now.iso8601 + }] + ) + ) +end + def mark_schedule_daemon_running(logs_dir, pid) FileUtils.mkdir_p(logs_dir) now = Time.now.iso8601 @@ -385,6 +405,38 @@ def write_smoke_script(path) }); page.on("pageerror", (error) => errors.push(error.message)); + async function captureVisibleComposer(filename) { + if (!captureDir) return; + const composer = page.locator("#composer:not([hidden])"); + await composer.waitFor({ state: "visible", timeout: 10_000 }); + const bounds = await composer.boundingBox(); + if (!bounds) throw new Error("Visible composer had no screenshot bounds"); + const viewport = page.viewportSize(); + const padding = 36; + const x = Math.max(0, bounds.x - padding); + const y = Math.max(0, bounds.y - padding); + const clip = { + x, + y, + width: Math.min(viewport.width, bounds.x + bounds.width + padding) - x, + height: Math.min(viewport.height, bounds.y + bounds.height + padding) - y, + }; + await page.screenshot({ path: path.join(captureDir, filename), clip }); + } + + async function ensureSkillAutocompleteVisible() { + for (let attempt = 0; attempt < 20; attempt += 1) { + if (await page.locator("[data-skill-autocomplete]:not(.hidden)").isVisible()) return; + await page.evaluate(() => { + const prompt = document.querySelector("#prompt-input"); + prompt?.focus(); + if (prompt) refreshSkillAutocomplete(prompt); + }); + await page.waitForTimeout(100); + } + throw new Error("Skill autocomplete did not become visible"); + } + async function assertResourceCatalogCompatibility() { const catalogTimings = await page.evaluate(async () => { const samples = []; @@ -1642,7 +1694,8 @@ def write_smoke_script(path) await agentsRegressionPage.close(); const prContextPage = await browser.newPage({ viewport: { width: 390, height: 844 } }); - await prContextPage.goto(`${baseUrl}/ui#agent/${encodeURIComponent(agentKey)}`, { waitUntil: "networkidle" }); + const prContextAgentKey = unscheduledAgentKey; + await prContextPage.goto(`${baseUrl}/ui#agent/${encodeURIComponent(prContextAgentKey)}`, { waitUntil: "networkidle" }); await prContextPage.waitForSelector("#prompt-input", { state: "visible", timeout: 10_000 }); await prContextPage.evaluate((key) => { const agent = findAgent(key); @@ -1652,12 +1705,184 @@ def write_smoke_script(path) ]; state.renderedViewHtml = ""; render(); - }, agentKey); + }, prContextAgentKey); await prContextPage.click("#header-more-button"); - await prContextPage.waitForSelector(`[data-open-agent-pr-diffs='${agentKey}']`, { state: "visible", timeout: 10_000 }); - await prContextPage.click(`[data-open-agent-pr-diffs='${agentKey}']`); + await prContextPage.waitForSelector(`[data-open-agent-pr-diffs='${prContextAgentKey}']`, { state: "visible", timeout: 10_000 }); + await prContextPage.click(`[data-open-agent-pr-diffs='${prContextAgentKey}']`); await prContextPage.waitForSelector(".agent-pr-diff-shell", { state: "visible", timeout: 10_000 }); await prContextPage.waitForSelector(".agent-pr-diff-shell .empty-state", { state: "visible", timeout: 10_000 }); + await prContextPage.click("[data-refresh-pr-diff]"); + await prContextPage.waitForSelector("[data-select-pr-diff-line]", { state: "visible", timeout: 10_000 }); + const selectableLines = prContextPage.locator("[data-select-pr-diff-line]"); + if (await selectableLines.count() !== 2) throw new Error("PR detail did not expose selectable changed lines"); + const firstLine = selectableLines.nth(0); + const secondLine = selectableLines.nth(1); + if (!(await firstLine.getAttribute("aria-label"))?.includes("lib/example.rb")) { + throw new Error("PR diff line control is missing an accessible file and line label"); + } + const rangeLimitMessage = await prContextPage.evaluate((key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diffKey = pullRequestDiffKey(key, pullRequestId); + const diff = state.pullRequestDiffs[diffKey]; + const originalLines = diff.files[0].hunks[0].lines; + diff.files[0].hunks[0].lines = Array.from({ length: 101 }, (_, index) => ({ + kind: "context", old_number: index + 1, new_number: index + 1, content: `line ${index + 1}`, + })); + const selection = pullRequestDiffSelection(key, pullRequestId, diff.snapshot_id); + selection.anchor = pullRequestDiffLine(diff, "lib/example.rb", 0, 0); + selection.lines = [selection.anchor]; + selectPullRequestDiffLine({ dataset: { + agentKey: key, + pullRequestId, + snapshotId: diff.snapshot_id, + reviewLinePath: "lib/example.rb", + reviewLineHunk: "0", + reviewLineIndex: "100", + } }, true); + const message = document.querySelector("#growl")?.innerText || ""; + diff.files[0].hunks[0].lines = originalLines; + selection.anchor = null; + selection.lines = []; + state.renderedViewHtml = ""; + render(); + return message; + }, prContextAgentKey); + if (!rangeLimitMessage.includes("at most 100 contiguous lines")) { + throw new Error(`PR range limit was not enforced visibly: ${JSON.stringify(rangeLimitMessage)}`); + } + const diffErrorState = await prContextPage.evaluate((key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diffKey = pullRequestDiffKey(key, pullRequestId); + const original = state.pullRequestDiffs[diffKey]; + state.pullRequestDiffs[diffKey] = { error: "stale fixture diff", files: [] }; + state.renderedViewHtml = ""; + render(); + const visible = document.querySelector(".ui-alert")?.innerText || ""; + state.pullRequestDiffs[diffKey] = original; + state.renderedViewHtml = ""; + render(); + return visible; + }, prContextAgentKey); + if (!diffErrorState.includes("Diff unavailable") || !diffErrorState.includes("stale fixture diff")) { + throw new Error(`PR diff errors are not visible: ${JSON.stringify(diffErrorState)}`); + } + await firstLine.click(); + await secondLine.click({ modifiers: ["Shift"] }); + const selectedRange = await prContextPage.evaluate(() => ({ + selected: document.querySelectorAll("[data-select-pr-diff-line]:checked").length, + selectionText: document.querySelector("[data-pr-context-selection]")?.innerText, + overflow: document.documentElement.scrollWidth > document.documentElement.clientWidth, + })); + if (selectedRange.selected !== 2 || !selectedRange.selectionText?.includes("2 lines selected") || selectedRange.overflow) { + throw new Error(`PR line range selection is not usable on mobile: ${JSON.stringify(selectedRange)}`); + } + await prContextPage.evaluate(() => document.querySelector("#growl")?.classList.add("hidden")); + if (captureDir) await prContextPage.screenshot({ path: path.join(captureDir, "pr-context-selected-mobile.png"), fullPage: true }); + await prContextPage.keyboard.press("Escape"); + if (await prContextPage.locator("[data-select-pr-diff-line]:checked").count()) { + const escapeState = await prContextPage.evaluate((key) => ({ + route: parseRoute(), + selections: state.pullRequestDiffSelections, + speech: Boolean(state.speechRecognition), + key, + }), prContextAgentKey); + throw new Error(`Escape did not clear the active PR diff range: ${JSON.stringify(escapeState)}`); + } + await firstLine.click(); + await secondLine.click({ modifiers: ["Shift"] }); + await prContextPage.evaluate(async () => refresh({ force: true, forceConversation: true })); + await prContextPage.waitForSelector("[data-select-pr-diff-line]:checked", { state: "visible", timeout: 10_000 }); + if (await prContextPage.locator("[data-select-pr-diff-line]:checked").count() !== 2) { + throw new Error("PR line selection did not survive a conversation refresh"); + } + await prContextPage.click("[data-attach-pr-diff-selection]"); + await prContextPage.waitForSelector("[data-pending-pr-context]", { state: "visible", timeout: 10_000 }); + const pendingContext = await prContextPage.evaluate(() => ({ + text: document.querySelector("[data-pending-pr-context]")?.innerText, + focused: document.activeElement?.id === "prompt-input", + })); + if (!pendingContext.text?.includes("example/web#123") || + !pendingContext.text?.includes("lib/example.rb") || + !pendingContext.text?.includes("mixed sides") || + !pendingContext.focused) { + throw new Error(`Attached PR context is incomplete or did not focus the composer: ${JSON.stringify(pendingContext)}`); + } + await prContextPage.click("[data-remove-pr-context]"); + if (await prContextPage.locator("[data-pending-pr-context]").count()) { + throw new Error("Pending PR context could not be removed"); + } + await firstLine.click(); + await secondLine.click({ modifiers: ["Shift"] }); + await prContextPage.click("[data-attach-pr-diff-selection]"); + await firstLine.click(); + await secondLine.click({ modifiers: ["Shift"] }); + await prContextPage.click("[data-attach-pr-diff-selection]"); + if (await prContextPage.locator("[data-pending-pr-context]").count() !== 1 || + !(await prContextPage.locator("#growl").innerText()).includes("already attached")) { + throw new Error("Duplicate PR context was not rejected visibly"); + } + await prContextPage.evaluate((key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; + diff.snapshot_id = "newer-snapshot"; + state.renderedViewHtml = ""; + render(); + }, prContextAgentKey); + if (!(await prContextPage.locator("[data-pending-pr-context]").innerText()).includes("outdated")) { + throw new Error("Stale pending PR context is not called out before submission"); + } + await prContextPage.evaluate((key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; + diff.snapshot_id = document.querySelector("[data-pending-pr-context]")?.dataset.pendingPrContext + ? pendingPullRequestContextsFor(key)[0].snapshot_id + : diff.snapshot_id; + state.renderedViewHtml = ""; + render(); + }, prContextAgentKey); + await prContextPage.setViewportSize({ width: 1280, height: 800 }); + await prContextPage.evaluate(() => document.querySelector("#growl")?.classList.add("hidden")); + if (captureDir) await prContextPage.screenshot({ path: path.join(captureDir, "pr-context-pending-desktop.png"), fullPage: true }); + await prContextPage.evaluate(() => { + window.__originalPendingAttachmentPayloads = pendingAttachmentPayloads; + pendingAttachmentPayloads = async () => new Promise((resolve) => { + window.__releasePromptAttachmentPayloads = () => resolve([]); + }); + }); + await prContextPage.fill("#prompt-input", "Explain the selected change."); + await prContextPage.click("#composer button[type='submit']"); + await prContextPage.waitForFunction(() => typeof window.__releasePromptAttachmentPayloads === "function"); + await firstLine.click(); + await prContextPage.click("[data-attach-pr-diff-selection]"); + if (await prContextPage.locator("[data-pending-pr-context]").count() !== 2) { + throw new Error("PR context could not be queued while another prompt payload was encoding"); + } + await prContextPage.evaluate(() => window.__releasePromptAttachmentPayloads()); + await prContextPage.waitForFunction( + () => arrayValue(state.conversations[parseRoute().key]?.blocks) + .some((block) => block.content?.includes("[TYCHO_PR_DIFF_CONTEXT]")), + null, + { timeout: 10_000 } + ); + const sentContext = await prContextPage.evaluate((key) => { + pendingAttachmentPayloads = window.__originalPendingAttachmentPayloads; + const message = arrayValue(state.conversations[key]?.blocks).find((block) => block.content?.includes("[TYCHO_PR_DIFF_CONTEXT]")); + return { + content: message?.content || "", + pending: pendingPullRequestContextsFor(key).map((context) => context.lines.length), + }; + }, prContextAgentKey); + if (!sentContext.content.includes('"repository":"example/web"') || + !sentContext.content.includes('"path":"lib/example.rb"') || + !sentContext.content.includes('"side":"left"') || + !sentContext.content.includes('"side":"right"') || + JSON.stringify(sentContext.pending) !== "[1]") { + throw new Error(`Sent PR context lost metadata or consumed a later range: ${JSON.stringify(sentContext)}`); + } await prContextPage.close(); const pausedReviewPage = await browser.newPage({ viewport: { width: 390, height: 844 } }); @@ -1788,13 +2013,44 @@ def write_smoke_script(path) await reviewPage.close(); } - await page.goto(`${baseUrl}/ui#agent/${encodeURIComponent(agentKey)}`, { waitUntil: "networkidle" }); + const speechAgentKey = await page.evaluate(async () => { + const data = await apiPost("/agents", { + project_key: "web", + template_key: "custom", + name: "Speech Smoke Agent", + prompt: "Test speech input.", + agent: "codex", + }); + state.agents.push(data.agent); + return data.agent.key; + }); + await page.goto(`${baseUrl}/ui#agent/${encodeURIComponent(speechAgentKey)}`, { waitUntil: "networkidle" }); await page.waitForSelector("#prompt-input", { state: "visible", timeout: 10_000 }); await page.evaluate(() => { window.SpeechRecognition = class SmokeSpeechRecognition { + constructor() { + window.__speechRecognitionInstances ||= []; + window.__speechRecognitionInstances.push(this); + } start() { window.__speechRecognitionStarted = (window.__speechRecognitionStarted || 0) + 1; } - stop() {} + stop() { + this.stopped = true; + if (this.finalOnStop) { + this.onresult?.({ + resultIndex: 0, + results: [{ isFinal: true, 0: { transcript: this.finalOnStop } }], + }); + } + this.onend?.(); + } + abort() { + this.aborted = true; + this.onerror?.({ error: "aborted" }); + } }; + if (state.timer) window.clearTimeout(state.timer); + state.timer = null; + state.pollDeferredUntil = Date.now() + 60_000; state.renderedViewHtml = ""; render(); const hiddenComposer = document.createElement("form"); @@ -1805,9 +2061,33 @@ def write_smoke_script(path) els.view.append(hiddenComposer); document.activeElement?.blur(); }); + await page.waitForTimeout(1200); + const speechIconContract = await page.evaluate(() => ({ + icon: Boolean(document.querySelector('[data-toggle-speech-mode] path[d^="M12 2a3"]')), + state: document.querySelector("[data-toggle-speech-mode]")?.dataset.state, + label: document.querySelector("[data-toggle-speech-mode]")?.getAttribute("aria-label"), + })); + if (!speechIconContract.icon || speechIconContract.state !== "idle" || !speechIconContract.label?.includes("Start speech recognition")) { + throw new Error(`Speech idle control is not clear or accessible: ${JSON.stringify(speechIconContract)}`); + } + await captureVisibleComposer("speech-idle-icon-desktop.png"); await page.keyboard.press("Meta+Shift+."); + const listeningState = await page.evaluate(() => ({ + state: document.querySelector("[data-toggle-speech-mode]")?.dataset.state, + status: document.querySelector("[data-speech-mode-status]")?.textContent, + interimResults: state.speechRecognition?.interimResults, + })); + if (listeningState.state !== "listening" || !listeningState.status?.includes("Listening") || !listeningState.interimResults) { + throw new Error(`Speech listening state is incomplete: ${JSON.stringify(listeningState)}`); + } + await captureVisibleComposer("speech-listening-desktop.png"); const speechShortcutContract = await page.evaluate((key) => { const recognition = state.speechRecognition; + recognition?.onresult?.({ + resultIndex: 0, + results: [{ isFinal: false, 0: { transcript: "spoken" } }], + }); + const interim = document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"] #prompt-input`)?.value; recognition?.onresult?.({ resultIndex: 0, results: [{ isFinal: true, 0: { transcript: "spoken smoke" } }], @@ -1816,19 +2096,36 @@ def write_smoke_script(path) started: window.__speechRecognitionStarted, selectedAgentKey: state.speechComposerKey, focusedPrompt: document.activeElement?.id === "prompt-input" && document.activeElement?.closest("#composer")?.dataset.agentKey === key, + interim, transcript: document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"] #prompt-input`)?.value, + state: state.speechState, + status: document.querySelector("[data-speech-mode-status]")?.textContent, shortcutHint: document.querySelector("[data-toggle-speech-mode]")?.getAttribute("aria-label"), }; - }, agentKey); + }, speechAgentKey); if (speechShortcutContract.started !== 1 || - speechShortcutContract.selectedAgentKey !== agentKey || + speechShortcutContract.selectedAgentKey !== speechAgentKey || !speechShortcutContract.focusedPrompt || + speechShortcutContract.interim !== "spoken" || speechShortcutContract.transcript !== "spoken smoke" || + speechShortcutContract.state !== "processing" || + !speechShortcutContract.status?.includes("Transcript added") || !speechShortcutContract.shortcutHint?.includes("Shift+.")) { throw new Error(`Speech shortcut did not select and focus the visible composer: ${JSON.stringify(speechShortcutContract)}`); } - if (captureDir) await page.screenshot({ path: path.join(captureDir, "speech-shortcut-desktop.png") }); + await captureVisibleComposer("speech-transcribed-desktop.png"); await page.evaluate(() => stopSpeechMode()); + await page.click("#composer button[type='submit']"); + await page.waitForFunction( + (key) => arrayValue(state.conversations[key]?.blocks).some((block) => block.role === "user" && block.content === "spoken smoke"), + speechAgentKey, + { timeout: 10_000 } + ); + await page.waitForFunction((key) => !agentIsRunning(findAgent(key)), speechAgentKey, { timeout: 10_000 }); + await page.evaluate(() => { + state.renderedViewHtml = ""; + render(); + }); const speechGuardContract = await page.evaluate((key) => { const shortcut = (overrides = {}) => { let prevented = false; @@ -1897,16 +2194,128 @@ def write_smoke_script(path) externalInput.remove(); editable.remove(); return results; - }, agentKey); + }, speechAgentKey); if (speechGuardContract.textInput.handled || speechGuardContract.textInput.prevented || speechGuardContract.contentEditable.handled || speechGuardContract.contentEditable.prevented || speechGuardContract.repeat.handled || speechGuardContract.composing.handled || speechGuardContract.dialog.handled || speechGuardContract.dialog.prevented || - speechGuardContract.unsupported.handled || speechGuardContract.unsupported.prevented || + !speechGuardContract.unsupported.handled || !speechGuardContract.unsupported.prevented || !speechGuardContract.multiple.handled || !speechGuardContract.focusedVisibleComposer || - !speechGuardContract.disabledIgnored || speechGuardContract.activeKey !== agentKey) { + !speechGuardContract.disabledIgnored || speechGuardContract.activeKey !== speechAgentKey) { throw new Error(`Speech shortcut guards or composer selection failed: ${JSON.stringify(speechGuardContract)}`); } + const speechLifecycleContract = await page.evaluate((key) => { + const composer = document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"]`); + const input = composer.querySelector("#prompt-input"); + input.value = "before after"; + input.setSelectionRange(6, 6); + startSpeechMode(composer); + const firstRecognition = state.speechRecognition; + firstRecognition.onresult({ + resultIndex: 0, + results: [{ isFinal: true, 0: { transcript: "spoken words" } }], + }); + const insertedAtCaret = input.value; + startSpeechMode(composer); + const repeatedStopped = firstRecognition.stopped === true && !state.speechRecognition; + + input.value = "keep"; + input.setSelectionRange(input.value.length, input.value.length); + startSpeechMode(composer); + const cleanStopRecognition = state.speechRecognition; + cleanStopRecognition.onresult({ + resultIndex: 0, + results: [{ isFinal: false, 0: { transcript: "last words" } }], + }); + cleanStopRecognition.finalOnStop = "last words finalized"; + startSpeechMode(composer); + const cleanStopped = input.value; + + input.value = "keep this"; + input.setSelectionRange(4, 4); + startSpeechMode(composer); + const cancelRecognition = state.speechRecognition; + cancelRecognition.onresult({ + resultIndex: 0, + results: [{ isFinal: false, 0: { transcript: " temporary" } }], + }); + stopSpeechMode({ cancel: true }); + const cancelled = input.value; + + startSpeechMode(composer); + const permissionRecognition = state.speechRecognition; + permissionRecognition.onerror({ error: "not-allowed" }); + const permission = { + state: document.querySelector("[data-toggle-speech-mode]")?.dataset.state, + status: document.querySelector("[data-speech-mode-status]")?.textContent, + }; + + startSpeechMode(composer); + const networkRecognition = state.speechRecognition; + networkRecognition.onerror({ error: "network" }); + const network = { + state: document.querySelector("[data-toggle-speech-mode]")?.dataset.state, + status: document.querySelector("[data-speech-mode-status]")?.textContent, + }; + + startSpeechMode(composer); + const rerenderRecognition = state.speechRecognition; + state.renderedViewHtml = "force speech cleanup"; + render(); + const rerenderCleaned = rerenderRecognition.stopped === true && !state.speechRecognition; + + const navigationComposer = document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"]`); + startSpeechMode(navigationComposer); + const navigationRecognition = state.speechRecognition; + window.dispatchEvent(new HashChangeEvent("hashchange")); + const navigationCleaned = navigationRecognition.stopped === true && !state.speechRecognition; + + return { + insertedAtCaret, repeatedStopped, cleanStopped, cancelled, permission, network, + rerenderCleaned, navigationCleaned, + }; + }, speechAgentKey); + if (speechLifecycleContract.insertedAtCaret !== "before spoken words after" || + !speechLifecycleContract.repeatedStopped || + speechLifecycleContract.cleanStopped !== "keep last words finalized" || + speechLifecycleContract.cancelled !== "keep this" || + speechLifecycleContract.permission.state !== "error" || + !speechLifecycleContract.permission.status?.includes("permission denied") || + speechLifecycleContract.network.state !== "error" || + !speechLifecycleContract.network.status?.includes("network service") || + !speechLifecycleContract.rerenderCleaned || + !speechLifecycleContract.navigationCleaned) { + throw new Error(`Speech lifecycle, caret, permission, or cleanup failed: ${JSON.stringify(speechLifecycleContract)}`); + } + const unsupportedSpeechContract = await page.evaluate(() => { + const constructor = window.SpeechRecognition; + Object.defineProperties(window, { + SpeechRecognition: { configurable: true, value: undefined }, + webkitSpeechRecognition: { configurable: true, value: undefined }, + }); + state.renderedViewHtml = ""; + render(); + const button = document.querySelector("[data-toggle-speech-mode]"); + button.click(); + const result = { + disabled: button.disabled, + state: button.dataset.state, + status: document.querySelector("[data-speech-mode-status]")?.textContent, + growl: document.querySelector("#growl")?.innerText, + }; + Object.defineProperty(window, "SpeechRecognition", { configurable: true, value: constructor }); + state.pollDeferredUntil = 0; + schedule(); + state.renderedViewHtml = ""; + render(); + return result; + }); + if (unsupportedSpeechContract.disabled || unsupportedSpeechContract.state !== "unsupported" || + !unsupportedSpeechContract.status?.includes("unavailable") || + !unsupportedSpeechContract.growl?.includes("unavailable")) { + throw new Error(`Unsupported speech recognition is not actionable: ${JSON.stringify(unsupportedSpeechContract)}`); + } + await page.goto(`${baseUrl}/ui#agent/${encodeURIComponent(agentKey)}`, { waitUntil: "networkidle" }); await page.waitForSelector("#header-schedule-menu:not(.hidden)", { state: "visible", timeout: 10_000 }); const scheduleHeaderGeometry = await page.evaluate(() => { const scheduleTrigger = document.querySelector("[data-header-schedule-menu] > summary").getBoundingClientRect(); @@ -2214,10 +2623,14 @@ def write_smoke_script(path) const agent = findAgent(key); return agent && !agentIsRunning(agent); }, agentKey, { timeout: 10_000 }); + await page.evaluate(() => { + state.renderedViewHtml = ""; + render(); + }); await page.click("#prompt-input"); await page.fill("#prompt-input", "$rev"); await page.evaluate(() => refreshSkillAutocomplete(document.querySelector("#prompt-input"))); - await page.waitForSelector("[data-skill-autocomplete]:not(.hidden)", { state: "visible", timeout: 10_000 }); + await ensureSkillAutocompleteVisible(); if (!(await page.locator("[data-skill-autocomplete-rescan]").isVisible())) { throw new Error("Skill autocomplete re-scan command was not visible"); } @@ -2225,7 +2638,7 @@ def write_smoke_script(path) if (typeof refresh !== "function") throw new Error("refresh function is unavailable"); await refresh({ force: true, forceConversation: true }); }); - await page.waitForSelector("[data-skill-autocomplete]:not(.hidden)", { state: "visible", timeout: 10_000 }); + await ensureSkillAutocompleteVisible(); await page.waitForSelector("[data-skill-autocomplete-index]", { state: "visible", timeout: 10_000 }); const stillAutocompleting = await page.locator("#prompt-input").inputValue(); if (stillAutocompleting !== "$rev") { @@ -3551,6 +3964,8 @@ Dir.mktmpdir("tycho-remote-ui-smoke") do |dir| agent_key = create_agent(base_url) unscheduled_agent_key = create_agent(base_url, name: "Unscheduled Smoke Agent") associate_agent_schedule(fixture.fetch(:logs_dir), agent_key) + associate_agent_pull_request(fixture.fetch(:logs_dir), agent_key) + associate_agent_pull_request(fixture.fetch(:logs_dir), unscheduled_agent_key) wait_for_resource_agents(base_url, [agent_key, unscheduled_agent_key]) node_dir = File.join(dir, "node") install_playwright!(node_dir) diff --git a/docs/reports/usage-2026-07-06-to-2026-08-06.md b/docs/reports/usage-2026-07-06-to-2026-08-06.md deleted file mode 100644 index 4b8ec7a..0000000 --- a/docs/reports/usage-2026-07-06-to-2026-08-06.md +++ /dev/null @@ -1,57 +0,0 @@ -# Managed-agent usage: 2026-07-06 through 2026-08-06 - -This report covers calendar dates July 6 through August 6 in `Asia/Jakarta`, expressed as the half-open interval `[2026-07-06, 2026-08-07)`. It was rebuilt after an idempotent historical backfill and queries only the normalized metrics interface. The reporting query does not scan raw logs. - -Reproduce it with: - -```bash -bundle exec bin/tycho metrics backfill --timezone Asia/Jakarta --json -bundle exec bin/tycho metrics query \ - --from 2026-07-06 \ - --to 2026-08-07 \ - --timezone Asia/Jakarta \ - --json -``` - -The checked-in query envelope and exact summary are in [usage-2026-07-06-to-2026-08-06.query.json](./usage-2026-07-06-to-2026-08-06.query.json). - -## Summary - -| Measure | Result | -|---|---:| -| Run starts | 2,232 | -| Managed agents | 193 | -| Distinct native sessions | 193 | -| Runs without a native session ID | 72 | -| Average runs per native session | 11.19 | -| Known estimated cost | $2,850.0494 | -| Priced runs | 1,499 | -| Unpriced runs | 733 | -| Priced run coverage | 67.2% | -| Fully priced sessions | 93 | -| Unpriced or partially priced sessions | 100 | -| Median priced session cost | $6.8795 | -| Maximum priced session cost | $407.1918 | - -Costs are estimates, not invoices. The known total sums runs with a reported or reproducibly priced estimate. Median and maximum include a session only when every run has a known cost; missing non-cost metadata does not exclude a priced session. Average runs per session counts only the 2,160 starts associated with a native session, not the 72 orphan starts. - -## Provider coverage - -| Adapter | Runs | Native sessions | Priced runs | Known estimated cost | -|---|---:|---:|---:|---:| -| Claude-compatible | 1,480 | 97 | 1,472 | $2,823.9456 | -| Codex | 669 | 90 | 19 | $25.9040 | -| OpenCode | 8 | 3 | 8 | $0.1998 | -| Unknown harness | 75 | 3 | 0 | unknown | - -Most unpriced Codex history lacks an archived configured model, so Tycho preserves its token telemetry without choosing a price. The 75 unknown-harness starts have durable timestamps but no provider event shape; 72 also lack a native session ID. They remain run starts with explicit unknown reasons instead of disappearing or becoming zero-cost runs. - -## Reconciliation with the prior report - -The prior ad-hoc report cited 2,156 run starts and 181 native sessions. The normalized interface reports 2,232 starts and 193 sessions: - -- The run count is 76 higher. Seventy-five are telemetry-empty starts that the old provider-event scan omitted; the normalized schema retains them with unknown harness/status/price fields. One residual run reflects a difference in the old scan or its exact cutoff. The prior report artifact is not present in this repository, so attributing that last run would require recreating the old raw-log query; this report deliberately does not do that. -- The native-session count is 12 higher. Three belong to runs whose exact session IDs survived even though their harness did not. The remaining difference comes from counting exact provider-scoped native IDs rather than using managed-agent or priced-run proxies. Managed-agent count is reported separately to prevent the prior conflation. -- The earlier finding that archived Claude-compatible history lacked exact configured model labels remains visible as incomplete provenance. Observed `modelUsage` attribution is retained where emitted, but Tycho does not promote it into a guessed configured model or invent a price. - -The new figures are reproducible from the persisted v1 metric records. Backfill was rerun after completion and returned zero creates/updates, confirming idempotence. diff --git a/docs/reports/usage-2026-07-06-to-2026-08-06.query.json b/docs/reports/usage-2026-07-06-to-2026-08-06.query.json deleted file mode 100644 index f1b142f..0000000 --- a/docs/reports/usage-2026-07-06-to-2026-08-06.query.json +++ /dev/null @@ -1,28 +0,0 @@ -{ - "interface": "tycho metrics query", - "schema_version": 1, - "query": { - "from": "2026-07-06", - "to": "2026-08-07", - "timezone": "Asia/Jakarta", - "range_semantics": "from_inclusive_to_exclusive", - "filters": {} - }, - "command": "bundle exec bin/tycho metrics query --from 2026-07-06 --to 2026-08-07 --timezone Asia/Jakarta --json", - "summary": { - "run_starts": 2232, - "managed_agents": 193, - "distinct_native_sessions": 193, - "runs_without_native_session": 72, - "average_runs_per_session": 11.191709844559586, - "known_estimated_cost_usd": 2850.0493570296, - "median_priced_session_cost_usd": 6.8794935, - "max_priced_session_cost_usd": 407.19176695, - "priced_run_count": 1499, - "unpriced_run_count": 733, - "priced_run_coverage": 0.671594982078853, - "priced_session_count": 93, - "unpriced_or_partially_priced_session_count": 100, - "cost_semantics": "estimate_not_invoice" - } -} diff --git a/lib/hq/remote_server.rb b/lib/hq/remote_server.rb index 5b02507..c54da7f 100644 --- a/lib/hq/remote_server.rb +++ b/lib/hq/remote_server.rb @@ -1453,6 +1453,7 @@ class RemoteService ".woff2" => "font/woff2" }.freeze MAX_PULL_REQUEST_INBOX_ITEMS = 100 + MAX_PROMPT_PULL_REQUEST_CONTEXTS = 5 IMAGE_CONTENT_TYPES = { ".gif" => "image/gif", ".heic" => "image/heic", @@ -2622,8 +2623,10 @@ def mark_agent_read(key) def submit_prompt(key, attrs) target = find_agent!(key) + pull_request_context = render_prompt_pull_request_contexts(target, attrs) attachments = import_prompt_attachments(target, attrs) text = prompt_text(attrs, attachments:) + text = [text, pull_request_context].reject(&:empty?).join("\n") target.add_user_message!( text, attachments: @@ -3767,6 +3770,27 @@ def prompt_text(attrs, attachments:) raise Error.new("prompt is required") end + def render_prompt_pull_request_contexts(target, attrs) + contexts = attrs["pull_request_contexts"] + return "" unless contexts.is_a?(Array) && contexts.any? + if contexts.length > MAX_PROMPT_PULL_REQUEST_CONTEXTS + raise Error.new("Attach at most #{MAX_PROMPT_PULL_REQUEST_CONTEXTS} pull request ranges.", status: 400) + end + + rendered = contexts.map do |raw| + raise Error.new("Pull request context must be an object.", status: 400) unless raw.is_a?(Hash) + + reference = pull_request_reference!(target, raw["pull_request_id"]) + snapshot = @pull_request_diff_store.fetch(reference.id) + raise Error.new("Fetch the pull request diff before attaching lines.", status: 409) unless snapshot + + PullRequestSelection.render(snapshot, raw) + rescue PullRequestSelection::Error => e + raise Error.new(e.message, status: 409) + end + rendered.join("\n") + end + def inquiry_answer_with_feedback(answer, feedback, supplied:) parsed = JSON.parse(answer) return [answer, false] unless parsed.is_a?(Hash) diff --git a/lib/hq/remote_ui/assets/app.css b/lib/hq/remote_ui/assets/app.css index 0e6aaee..cb83209 100644 --- a/lib/hq/remote_ui/assets/app.css +++ b/lib/hq/remote_ui/assets/app.css @@ -3438,6 +3438,23 @@ select { .diff-line.selected { outline: 1px solid var(--accent); outline-offset: -1px; } +.diff-line.selectable { + cursor: pointer; +} + +.diff-line.selectable:hover, +.diff-line.selectable:focus-within { + background: color-mix(in srgb, var(--accent) 12%, transparent); +} + +.diff-line-select { + align-self: center; + width: 16px; + height: 16px; + margin: 0; + accent-color: var(--accent); +} + .diff-line.added { background: color-mix(in srgb, var(--ok) 14%, transparent); } @@ -3474,7 +3491,71 @@ select { white-space: pre; } +.pr-context-selection-bar { + position: sticky; + top: 0; + z-index: 3; + display: flex; + align-items: center; + justify-content: space-between; + gap: 12px; + border: 1px solid var(--border); + border-radius: 10px; + background: color-mix(in srgb, var(--panel) 94%, transparent); + padding: 10px 12px; + box-shadow: 0 8px 22px rgba(0, 0, 0, 0.18); +} + +.pr-context-selection-bar.has-selection { + border-color: var(--accent); +} + +.pr-context-selection-copy, +.pr-context-selection-actions { + display: flex; + align-items: center; + gap: 8px; +} + +.pr-context-selection-copy > span { + display: grid; + gap: 2px; +} + +.pr-context-selection-copy > span > span { + color: var(--muted); + font-size: 0.82rem; +} + +.pending-pr-contexts { + display: grid; + gap: 8px; + border: 1px solid color-mix(in srgb, var(--accent) 55%, var(--border)); + border-radius: 10px; + background: color-mix(in srgb, var(--accent) 7%, var(--panel)); + padding: 10px; +} + +.pending-pr-context.stale { + border-color: var(--danger); +} + +.pending-pr-context .pending-attachment-thumb { + color: var(--accent); +} + @media (max-width: 560px) { + .pr-context-selection-bar, + .pr-context-selection-actions { + align-items: stretch; + flex-direction: column; + } + + .pr-context-selection-actions .ui-button { + justify-content: center; + width: 100%; + } + .diff-toolbar { align-items: stretch; flex-direction: column; @@ -5845,6 +5926,10 @@ body:has(.inquiry-form-full-screen) { .agent-running-indicator .ui-icon { animation: none; } + + .speech-mode-button { + animation: none !important; + } } .skill-toggle-button, @@ -5861,6 +5946,59 @@ body:has(.inquiry-form-full-screen) { line-height: 1; } +.speech-mode-control { + position: relative; + display: inline-flex; + align-items: center; + gap: 6px; + min-width: 0; +} + +.speech-mode-button[data-state="listening"] { + color: var(--danger); + box-shadow: 0 0 0 3px color-mix(in srgb, var(--danger) 18%, transparent); + animation: speech-listening-pulse 1.4s ease-in-out infinite; +} + +.speech-mode-button[data-state="processing"] { + color: var(--info); +} + +.speech-mode-button[data-state="error"], +.speech-mode-button[data-state="unsupported"] { + color: var(--muted); +} + +.speech-mode-status { + max-width: 180px; + color: var(--muted); + font-size: 0.78rem; + line-height: 1.2; +} + +.speech-mode-control[data-state="error"] .speech-mode-status { + color: var(--danger); +} + +@keyframes speech-listening-pulse { + 50% { transform: scale(1.06); } +} + +@media (max-width: 640px) { + .speech-mode-status { + position: absolute; + right: 0; + bottom: calc(100% + 6px); + z-index: 4; + max-width: min(260px, 72vw); + border: 1px solid var(--border); + border-radius: 8px; + background: var(--panel); + padding: 6px 8px; + box-shadow: 0 8px 22px rgba(0, 0, 0, 0.24); + } +} + .attachment-toggle-button { color: var(--info); } diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index 2a5bc59..b397fea 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -11,6 +11,10 @@ const PROMPT_ATTACHMENT_LIMITS = { maxFiles: 5, maxBytes: 10 * 1024 * 1024, }; +const PROMPT_PULL_REQUEST_CONTEXT_LIMITS = { + maxContexts: 5, + maxLines: 100, +}; const CLIPBOARD_ATTACHMENT_EXTENSIONS = { "application/json": ".json", "application/pdf": ".pdf", @@ -543,6 +547,13 @@ const ICONS = { `, + microphone: ` + + `, x: ` ${escapeHtml([diff.head_sha ? `head ${shortSha(diff.head_sha)}` : "", diff.fetched_at ? `fetched ${timeShort(diff.fetched_at)}` : ""].filter(Boolean).join(" / "))} + ${renderPullRequestLineSelection(agent, item, diff, selection)}
- ${files.length ? files.map((file, index) => renderProjectDiffFile(file, index, { openAll: expandAll })).join("") : emptyState("No file changes", "This pull request diff snapshot has no textual file changes.")} + ${files.length ? files.map((file, index) => renderProjectDiffFile(file, index, { + openAll: expandAll, + pullRequestContext: { agentKey: agent.key, pullRequestId: item.id, snapshotId: diff.snapshot_id || "", selectedLines: selection.lines }, + })).join("") : emptyState("No file changes", "This pull request diff snapshot has no textual file changes.")} +
+ `; +} + +function renderPullRequestLineSelection(agent, item, diff, selection) { + const count = selection.lines.length; + return ` +
+ + ${iconSvg("paperclip")} + ${count ? `${count} line${count === 1 ? "" : "s"} selected` : "Attach diff context"}${count ? "This immutable range is ready to attach." : "Select a line, then Shift+select another line for a contiguous range."} + + + ${count ? `` : ""} + +
`; } @@ -5469,6 +5509,7 @@ function renderAgentComposer(agent, skills, options = {}) { ${renderPromptAttachmentInput(agent)} ${renderPendingAttachments(agent)} + ${renderPendingPullRequestContexts(agent)} ${renderAgentAttachments(agent)} @@ -5812,29 +5825,41 @@ function scheduleAgentReading(agent) { }, 1200); } -function agentComposerAction(agent) { +function agentComposerAction(agent, sending = false) { if (agentIsRunning(agent)) { return ``; } - return ``; + return ``; } function renderSpeechModeButton(agent) { const available = speechModeAvailable(); const active = state.speechComposerKey === agent.key && Boolean(state.speechRecognition); - const ownsStatus = state.speechStatusComposerKey === agent.key; - const speechState = active || ownsStatus ? state.speechState : available ? "idle" : "unsupported"; + const speechState = speechModeStateForAgent(agent); const disabled = agentIsRunning(agent); - const label = active ? "Stop speech recognition" : available ? "Start speech recognition" : "Speech recognition unavailable"; + const label = speechModeControlLabel(speechState, active, available); const hint = `${label} (${speechModeShortcutLabel()})`; const status = speechModeStatus(speechState, state.speechMessage); - return ` - - ${escapeHtml(status)} + return ` + + ${escapeHtml(status)} `; } +function speechModeControlLabel(speechState, active, available) { + if (active) return "Stop speech recognition"; + if (speechState === "error") return `Speech recognition error. ${state.speechMessage || "Try again."} Retry speech recognition`; + return available ? "Start speech recognition" : "Speech recognition unavailable. Use the keyboard instead"; +} + +function speechModeStateForAgent(agent) { + const available = speechModeAvailable(); + const active = state.speechComposerKey === agent.key && Boolean(state.speechRecognition); + const ownsStatus = state.speechStatusComposerKey === agent.key; + return active || ownsStatus ? state.speechState : available ? "idle" : "unsupported"; +} + function speechModeStatus(value, message = "") { if (message) return message; return { @@ -5943,17 +5968,18 @@ function updateSpeechModeControls() { const available = speechModeAvailable(); const ownsStatus = button.closest("#composer")?.dataset.agentKey === state.speechStatusComposerKey; const value = active || ownsStatus ? state.speechState : available ? "idle" : "unsupported"; - const label = active ? "Stop speech recognition" : available ? "Start speech recognition" : "Speech recognition unavailable"; + const label = speechModeControlLabel(value, active, available); button.setAttribute("aria-label", `${label} (${speechModeShortcutLabel()})`); button.setAttribute("title", `${label} (${speechModeShortcutLabel()})`); button.setAttribute("aria-pressed", active ? "true" : "false"); button.dataset.state = value; const control = button.closest("[data-speech-mode-control]"); if (control) control.dataset.state = value; + const composer = button.closest("#composer"); + if (composer) composer.dataset.speechState = value; const status = control?.querySelector("[data-speech-mode-status]"); if (status) { status.textContent = speechModeStatus(value, active || ownsStatus ? state.speechMessage : ""); - status.classList.toggle("sr-only", value === "idle"); } }); } @@ -6768,11 +6794,43 @@ function renderProjectDiffFile(file, index, options = {}) { function renderDiffHunk(hunk, index, options = {}) { const lines = Array.isArray(hunk.lines) ? hunk.lines : []; + const selectedLines = arrayValue(options.pullRequestContext?.selection?.lines) + .filter((line) => line.path === options.path && line.hunk_index === index); + const finalSelectedLineIndex = selectedLines.length + ? Math.max(...selectedLines.map((line) => Number(line.line_index))) + : -1; return `
${escapeHtml(hunk.header || "@@")}
- ${lines.map((line, lineIndex) => renderDiffLine(line, lineIndex, { ...options, hunkIndex: index })).join("")} + ${lines.map((line, lineIndex) => `${renderDiffLine(line, lineIndex, { ...options, hunkIndex: index })}${lineIndex === finalSelectedLineIndex ? renderPullRequestInlineComment(options.pullRequestContext, options.path, index) : ""}`).join("")} +
+
+ `; +} + +function renderPullRequestInlineComment(context, path, hunkIndex) { + const selection = context?.selection; + const lines = arrayValue(selection?.lines); + if (!lines.some((line) => line.path === path && line.hunk_index === hunkIndex)) return ""; + + const first = lines[0] || {}; + const last = lines[lines.length - 1] || first; + const firstNumber = first.new_number || first.old_number || "?"; + const lastNumber = last.new_number || last.old_number || firstNumber; + const range = firstNumber === lastNumber ? `line ${firstNumber}` : `lines ${firstNumber}–${lastNumber}`; + const comment = String(selection.comment || ""); + const disabled = comment.trim() && !context.selectionLocked ? "" : "disabled"; + return ` +
+
+ Commenting on ${escapeHtml(range)} + +
+ +
+ +
`; @@ -6794,7 +6852,7 @@ function renderDiffLine(line, lineIndex = 0, options = {}) { const container = selectable ? "label" : "div"; return ` <${container} class="diff-line ${escapeAttr(kind)}${selected ? " selected" : ""}${selectable ? " selectable" : ""}"> - ${selectable ? `` : ""} + ${selectable ? `` : ""} ${diffLineNumber(line.old_number)} ${diffLineNumber(line.new_number)} ${escapeHtml(marker)} @@ -6820,7 +6878,7 @@ function pullRequestDiffSelection(agentKey, pullRequestId, snapshotId = "") { const key = pullRequestDiffKey(agentKey, pullRequestId); const current = state.pullRequestDiffSelections[key]; if (!current || (snapshotId && current.snapshotId !== snapshotId)) { - state.pullRequestDiffSelections[key] = { snapshotId, anchor: null, lines: [] }; + state.pullRequestDiffSelections[key] = { snapshotId, anchor: null, lines: [], comment: "" }; } return state.pullRequestDiffSelections[key]; } @@ -6882,11 +6940,16 @@ function selectPullRequestDiffLine(target, extendRange = false) { } else if (selected.lines.some((item) => pullRequestLineKey(item) === pullRequestLineKey(line))) { selected.anchor = null; selected.lines = []; + selected.comment = ""; } else { selected.anchor = line; selected.lines = [line]; + selected.comment = ""; } render(); + if (selected.lines.length) { + window.requestAnimationFrame(() => document.querySelector("[data-pr-context-comment]")?.focus({ preventScroll: true })); + } } function pendingPullRequestContextsFor(agentKey) { @@ -6900,20 +6963,35 @@ function pullRequestContextFingerprint(context) { return [context.pull_request_id, context.snapshot_id, ...arrayValue(context.lines).map(pullRequestLineKey)].join("|"); } -function attachSelectedPullRequestLines(agentKey, pullRequestId) { +function selectedPullRequestCommentContext(agentKey, pullRequestId) { const diff = state.pullRequestDiffs[pullRequestDiffKey(agentKey, pullRequestId)]; const selection = pullRequestDiffSelection(agentKey, pullRequestId, diff?.snapshot_id || ""); - if (!selection.lines.length) return; + const comment = String(selection.comment || "").trim(); + if (!selection.lines.length) return null; + if (!comment) { + showGrowl("Write a comment for the selected lines", "need"); + document.querySelector("[data-pr-context-comment]")?.focus({ preventScroll: true }); + return null; + } - const pending = pendingPullRequestContextsFor(agentKey); - const context = { + return { id: `pr-context-${Date.now()}-${Math.random().toString(16).slice(2)}`, pull_request_id: pullRequestId, snapshot_id: selection.snapshotId, repository: diff.repository || "repository", number: diff.number, + comment, lines: selection.lines.map((line) => ({ ...line })), }; +} + +function attachSelectedPullRequestLines(agentKey, pullRequestId) { + const diff = state.pullRequestDiffs[pullRequestDiffKey(agentKey, pullRequestId)]; + const selection = pullRequestDiffSelection(agentKey, pullRequestId, diff?.snapshot_id || ""); + const context = selectedPullRequestCommentContext(agentKey, pullRequestId); + if (!context) return; + + const pending = pendingPullRequestContextsFor(agentKey); if (pending.some((item) => pullRequestContextFingerprint(item) === pullRequestContextFingerprint(context))) { showGrowl("That pull request range is already attached", "need"); return; @@ -6926,14 +7004,57 @@ function attachSelectedPullRequestLines(agentKey, pullRequestId) { pending.push(context); selection.anchor = null; selection.lines = []; + selection.comment = ""; document.querySelectorAll("[data-select-pr-diff-line]").forEach((control) => { control.checked = false; }); render(); - window.requestAnimationFrame(() => { - const focusedComposer = document.querySelector(`#composer[data-agent-key="${CSS.escape(agentKey)}"]`)?.closest("details.focused-composer"); - if (focusedComposer) focusedComposer.open = true; - const prompt = document.querySelector(`#composer[data-agent-key="${CSS.escape(agentKey)}"] #prompt-input`); - prompt?.focus({ preventScroll: true }); - syncAgentDockLayout(); + showGrowl("Comment section added", "done"); +} + +function promptPullRequestContextPayload(context, options = {}) { + return { + pull_request_id: context.pull_request_id, + snapshot_id: context.snapshot_id, + ...(options.includeComment && context.comment ? { comment: context.comment } : {}), + lines: arrayValue(context.lines).map((line) => ({ + path: line.path, + hunk_index: line.hunk_index, + line_index: line.line_index, + })), + }; +} + +function sendSelectedPullRequestComment(agentKey, pullRequestId) { + const agent = findAgent(agentKey); + if (!agent || agentIsRunning(agent) || state.pendingComposerKeys.has(agentKey) || state.pendingPullRequestCommentKeys.has(agentKey)) { + showGrowl("Wait for the agent to finish before sending another comment", "need"); + return; + } + const context = selectedPullRequestCommentContext(agentKey, pullRequestId); + if (!context) return; + + const prompt = context.comment; + const pendingMessageId = addPendingConversationMessage( + agentKey, + pendingPromptMessageBlock(agentKey, prompt, [], [context]) + ); + state.pendingPullRequestCommentKeys.add(agentKey); + render(); + mutate(async () => { + await apiPost(`/agents/${encodeURIComponent(agentKey)}/messages`, { + prompt, + start: true, + attachments: [], + pull_request_contexts: [promptPullRequestContextPayload(context)], + }); + const selection = pullRequestDiffSelection(agentKey, pullRequestId, context.snapshot_id); + selection.anchor = null; + selection.lines = []; + selection.comment = ""; + state.pendingPullRequestCommentKeys.delete(agentKey); + removePendingConversationMessage(agentKey, pendingMessageId, { render: false }); + }).finally(() => { + state.pendingPullRequestCommentKeys.delete(agentKey); + removePendingConversationMessage(agentKey, pendingMessageId); }); } @@ -6952,6 +7073,7 @@ function clearActivePullRequestDiffSelection() { if (!selection.lines.length) return false; selection.anchor = null; selection.lines = []; + selection.comment = ""; document.querySelectorAll("[data-select-pr-diff-line]").forEach((control) => { control.checked = false; }); render(); return true; @@ -7834,6 +7956,7 @@ function renderPendingPullRequestContext(agentKey, context) { ${escapeHtml(`${context.repository}${context.number ? `#${context.number}` : ""} · ${first.path || "diff"}`)} ${escapeHtml(`${range} · ${side} · ${lines.length} selected${stale ? " · outdated; remove and reattach" : ""}`)} + ${context.comment ? `${escapeHtml(context.comment)}` : ""} @@ -11946,6 +12069,23 @@ els.view.addEventListener("click", (event) => { ); return; } + const sendPrDiffComment = event.target.closest("[data-send-pr-diff-comment]"); + if (sendPrDiffComment) { + sendSelectedPullRequestComment( + sendPrDiffComment.dataset.agentKey, + sendPrDiffComment.dataset.pullRequestId + ); + return; + } + const openPrContextComposer = event.target.closest("[data-open-pr-context-composer]"); + if (openPrContextComposer) { + const agentKey = openPrContextComposer.dataset.openPrContextComposer; + const focusedComposer = document.querySelector(`#composer[data-agent-key="${CSS.escape(agentKey)}"]`)?.closest("details.focused-composer"); + if (focusedComposer) focusedComposer.open = true; + document.querySelector(`#composer[data-agent-key="${CSS.escape(agentKey)}"] #prompt-input`)?.focus({ preventScroll: true }); + syncAgentDockLayout(); + return; + } const clearPrDiffSelection = event.target.closest("[data-clear-pr-diff-selection]"); if (clearPrDiffSelection) { const diff = state.pullRequestDiffs[pullRequestDiffKey( @@ -11959,6 +12099,7 @@ els.view.addEventListener("click", (event) => { ); selection.anchor = null; selection.lines = []; + selection.comment = ""; document.querySelectorAll("[data-select-pr-diff-line]").forEach((control) => { control.checked = false; }); render(); return; @@ -12845,6 +12986,21 @@ document.addEventListener("dragend", clearComposerDropTargets); document.addEventListener("drop", clearComposerDropTargets); els.view.addEventListener("input", (event) => { + if (event.target.matches("[data-pr-context-comment]")) { + const diff = state.pullRequestDiffs[pullRequestDiffKey( + event.target.dataset.agentKey, + event.target.dataset.pullRequestId + )]; + const selection = pullRequestDiffSelection( + event.target.dataset.agentKey, + event.target.dataset.pullRequestId, + diff?.snapshot_id || "" + ); + selection.comment = event.target.value; + event.target.closest("[data-pr-inline-comment]")?.querySelectorAll("[data-attach-pr-diff-selection], [data-send-pr-diff-comment]") + .forEach((button) => { button.disabled = !event.target.value.trim(); }); + return; + } if (event.target.matches("[data-review-filter]")) { state.pullRequestFilters.query = event.target.value; renderPullRequestInbox(); @@ -13178,13 +13334,7 @@ els.view.addEventListener("submit", (event) => { mutate(async () => { const attachments = await pendingAttachmentPayloads(key); const pullRequestContexts = submittedPullRequestContexts.map((context) => ({ - pull_request_id: context.pull_request_id, - snapshot_id: context.snapshot_id, - lines: context.lines.map((line) => ({ - path: line.path, - hunk_index: line.hunk_index, - line_index: line.line_index, - })), + ...promptPullRequestContextPayload(context, { includeComment: true }), })); await apiPost(`/agents/${encodeURIComponent(key)}/messages`, { prompt, diff --git a/test/remote_server_test.rb b/test/remote_server_test.rb index 991be13..b398f35 100644 --- a/test/remote_server_test.rb +++ b/test/remote_server_test.rb @@ -1157,6 +1157,7 @@ def assert_remote_prompt_accepts_pull_request_context "pull_request_contexts" => [{ "pull_request_id" => reference.id, "snapshot_id" => "composer-snapshot", + "comment" => "Explain why these two sides differ.", "lines" => [ { "path" => "lib/example.rb", "hunk_index" => 0, "line_index" => 0 }, { "path" => "lib/example.rb", "hunk_index" => 0, "line_index" => 1 } @@ -1167,9 +1168,24 @@ def assert_remote_prompt_accepts_pull_request_context "expected normal composer messages to include validated PR context") assert(content.include?('"path":"lib/example.rb"') && content.include?('"side":"left"') && content.include?('"side":"right"') && content.include?('"old_number":3') && - content.include?('"new_number":4'), + content.include?('"new_number":4') && + content.include?("Comment on this range:\nExplain why these two sides differ."), "expected PR context to carry repository file, side, and line metadata") + begin + service.submit_prompt(created[:key], + "prompt" => "Oversized comment.", + "pull_request_contexts" => [{ + "pull_request_id" => reference.id, "snapshot_id" => "composer-snapshot", + "comment" => "x" * ((8 * 1024) + 1), + "lines" => [{ "path" => "lib/example.rb", "hunk_index" => 0, "line_index" => 0 }] + }]) + raise "expected oversized PR comment to fail" + rescue HQ::RemoteServer::Error => e + assert(e.status == 400 && e.message.include?("at most 8 KB"), + "expected oversized PR comments to return a bounded input error") + end + begin asset_pattern = File.join(HQ::AGENT_LOGS_DIR, "assets", created[:key], "**", "*") asset_files_before = Dir.glob(asset_pattern).select { |path| File.file?(path) } From 49eabc073738302d50012c789d5f7807841fcd6c Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Sun, 9 Aug 2026 18:31:34 +0700 Subject: [PATCH 03/12] Polish diff selection and speech follow --- bin/remote-ui-smoke | 122 +++++++++++++++++++++++++++++++- lib/hq/remote_ui/assets/app.css | 43 +++++++++-- lib/hq/remote_ui/assets/app.js | 64 +++++++++++++++-- 3 files changed, 219 insertions(+), 10 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index d656277..766d98c 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -1783,6 +1783,17 @@ def write_smoke_script(path) if (!diffErrorState.includes("Diff unavailable") || !diffErrorState.includes("stale fixture diff")) { throw new Error(`PR diff errors are not visible: ${JSON.stringify(diffErrorState)}`); } + await prContextPage.evaluate((key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; + diff.files[0].hunks[0].lines[1].content = `new ${"wide-code-column ".repeat(24)}`; + diff.files[0].hunks[0].lines.push({ + kind: "context", old_number: 2, new_number: 2, content: "following unselected line", + }); + state.renderedViewHtml = ""; + render(); + }, prContextAgentKey); await firstLine.click(); await secondLine.click({ modifiers: ["Shift"] }); await prContextPage.waitForSelector("[data-pr-inline-comment]", { state: "visible", timeout: 10_000 }); @@ -1795,6 +1806,41 @@ def write_smoke_script(path) throw new Error(`PR line range selection is not usable on mobile: ${JSON.stringify(selectedRange)}`); } await prContextPage.fill("[data-pr-context-comment]", "Clarify this mixed-side range."); + await prContextPage.setViewportSize({ width: 1210, height: 640 }); + const landscapeInlineContract = await prContextPage.evaluate(() => { + const body = document.querySelector(".diff-file-body"); + const panel = document.querySelector("[data-pr-inline-comment]"); + const send = document.querySelector("[data-send-pr-diff-comment]"); + const add = document.querySelector("[data-attach-pr-diff-selection]"); + const selectedCheck = document.querySelector("[data-select-pr-diff-line]:checked + .diff-line-check"); + const emptyCheck = document.querySelector("[data-select-pr-diff-line]:not(:checked) + .diff-line-check"); + const nativeCheck = document.querySelector("[data-select-pr-diff-line]"); + const bodyRect = body?.getBoundingClientRect(); + const panelRect = panel?.getBoundingClientRect(); + const sendRect = send?.getBoundingClientRect(); + const addRect = add?.getBoundingClientRect(); + return { + bodyWidth: bodyRect?.width, + panelWidth: panelRect?.width, + panelInside: panelRect.left >= bodyRect.left - 1 && panelRect.right <= bodyRect.right + 1, + actionsInside: [sendRect, addRect].every((rect) => rect.left >= bodyRect.left - 1 && rect.right <= bodyRect.right + 1), + sendVisible: getComputedStyle(send).visibility !== "hidden" && sendRect.width > 0, + nativeHidden: getComputedStyle(nativeCheck).opacity === "0", + selectedMark: selectedCheck?.textContent, + emptyMark: emptyCheck?.textContent, + }; + }); + if (!landscapeInlineContract.panelInside || !landscapeInlineContract.actionsInside || + !landscapeInlineContract.sendVisible || !landscapeInlineContract.nativeHidden || + landscapeInlineContract.selectedMark !== "✓" || landscapeInlineContract.emptyMark !== "") { + throw new Error(`Landscape inline comment or minimal line selector failed: ${JSON.stringify(landscapeInlineContract)}`); + } + await prContextPage.evaluate(() => { + document.querySelector("#growl")?.classList.add("hidden"); + document.querySelector("[data-pr-inline-comment]")?.scrollIntoView({ block: "center", inline: "nearest" }); + }); + if (captureDir) await prContextPage.screenshot({ path: path.join(captureDir, "pr-context-selected-landscape.png"), fullPage: false }); + await prContextPage.setViewportSize({ width: 390, height: 844 }); await prContextPage.evaluate(() => document.querySelector("#growl")?.classList.add("hidden")); if (captureDir) await prContextPage.screenshot({ path: path.join(captureDir, "pr-context-selected-mobile.png"), fullPage: true }); await prContextPage.keyboard.press("Escape"); @@ -2262,7 +2308,7 @@ def write_smoke_script(path) !speechGuardContract.disabledIgnored || speechGuardContract.activeKey !== speechAgentKey) { throw new Error(`Speech shortcut guards or composer selection failed: ${JSON.stringify(speechGuardContract)}`); } - const speechLifecycleContract = await page.evaluate((key) => { + const speechLifecycleContract = await page.evaluate(async (key) => { const composer = document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"]`); const input = composer.querySelector("#prompt-input"); input.value = "before after"; @@ -2300,6 +2346,46 @@ def write_smoke_script(path) stopSpeechMode({ cancel: true }); const cancelled = input.value; + input.value = Array.from({ length: 24 }, (_, index) => `existing line ${index + 1}`).join("\\n"); + input.setSelectionRange(input.value.length, input.value.length); + input.scrollTop = 0; + startSpeechMode(composer); + const scrollingRecognition = state.speechRecognition; + scrollingRecognition.onresult({ + resultIndex: 0, + results: [{ isFinal: false, 0: { transcript: "spoken text at the end" } }], + }); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + const speechFollow = { + scrollTop: input.scrollTop, + scrollHeight: input.scrollHeight, + clientHeight: input.clientHeight, + }; + stopSpeechMode({ cancel: true }); + + const middleLines = Array.from({ length: 30 }, (_, index) => `middle line ${index + 1}`); + input.value = middleLines.join("\\n"); + input.dispatchEvent(new Event("input", { bubbles: true })); + const middleCaret = middleLines.slice(0, 10).join("\\n").length; + input.setSelectionRange(middleCaret, middleCaret); + input.scrollTop = 80; + const middleOriginalScrollTop = input.scrollTop; + startSpeechMode(composer); + const middleRecognition = state.speechRecognition; + middleRecognition.onresult({ + resultIndex: 0, + results: [{ isFinal: false, 0: { transcript: "spoken middle ".repeat(80) } }], + }); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + const middleFollow = { + originalScrollTop: middleOriginalScrollTop, + followedScrollTop: input.scrollTop, + maxScrollTop: input.scrollHeight - input.clientHeight, + }; + stopSpeechMode({ cancel: true }); + await new Promise((resolve) => requestAnimationFrame(resolve)); + middleFollow.restoredScrollTop = input.scrollTop; + startSpeechMode(composer); const permissionRecognition = state.speechRecognition; permissionRecognition.onerror({ error: "not-allowed" }); @@ -2332,13 +2418,19 @@ def write_smoke_script(path) return { insertedAtCaret, repeatedStopped, cleanStopped, cancelled, permission, network, - rerenderPreserved, navigationCleaned, + speechFollow, middleFollow, rerenderPreserved, navigationCleaned, }; }, speechAgentKey); if (speechLifecycleContract.insertedAtCaret !== "before spoken words after" || !speechLifecycleContract.repeatedStopped || speechLifecycleContract.cleanStopped !== "keep last words finalized" || speechLifecycleContract.cancelled !== "keep this" || + speechLifecycleContract.speechFollow.scrollTop <= 0 || + speechLifecycleContract.speechFollow.scrollTop + speechLifecycleContract.speechFollow.clientHeight < + speechLifecycleContract.speechFollow.scrollHeight - 2 || + speechLifecycleContract.middleFollow.followedScrollTop <= speechLifecycleContract.middleFollow.originalScrollTop || + speechLifecycleContract.middleFollow.followedScrollTop >= speechLifecycleContract.middleFollow.maxScrollTop - 2 || + Math.abs(speechLifecycleContract.middleFollow.restoredScrollTop - speechLifecycleContract.middleFollow.originalScrollTop) > 1 || speechLifecycleContract.permission.state !== "error" || !speechLifecycleContract.permission.status?.includes("permission denied") || !speechLifecycleContract.permission.label?.includes("Speech recognition error") || @@ -2349,6 +2441,32 @@ def write_smoke_script(path) !speechLifecycleContract.navigationCleaned) { throw new Error(`Speech lifecycle, caret, permission, or cleanup failed: ${JSON.stringify(speechLifecycleContract)}`); } + const speechFollowCapture = await page.evaluate(async (key) => { + const composer = document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"]`); + const input = composer.querySelector("#prompt-input"); + input.value = Array.from({ length: 24 }, (_, index) => `captured line ${index + 1}`).join("\\n"); + input.dispatchEvent(new Event("input", { bubbles: true })); + input.setSelectionRange(input.value.length, input.value.length); + input.scrollTop = 0; + startSpeechMode(composer); + state.speechRecognition.onresult({ + resultIndex: 0, + results: [{ isFinal: false, 0: { transcript: "spoken follow at the end" } }], + }); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + return { + value: input.value, + scrollTop: input.scrollTop, + scrollHeight: input.scrollHeight, + clientHeight: input.clientHeight, + }; + }, speechAgentKey); + if (!speechFollowCapture.value.endsWith("spoken follow at the end") || speechFollowCapture.scrollTop <= 0 || + speechFollowCapture.scrollTop + speechFollowCapture.clientHeight < speechFollowCapture.scrollHeight - 2) { + throw new Error(`Speech follow capture did not stay at the active transcript: ${JSON.stringify(speechFollowCapture)}`); + } + await captureVisibleComposer("speech-follow-scroll-desktop.png"); + await page.evaluate(() => stopSpeechMode({ cancel: true })); const unsupportedSpeechContract = await page.evaluate(() => { const constructor = window.SpeechRecognition; Object.defineProperties(window, { diff --git a/lib/hq/remote_ui/assets/app.css b/lib/hq/remote_ui/assets/app.css index 7525cf8..b7cd53a 100644 --- a/lib/hq/remote_ui/assets/app.css +++ b/lib/hq/remote_ui/assets/app.css @@ -3384,6 +3384,7 @@ select { .diff-file-body { display: grid; + container-type: inline-size; gap: 10px; min-width: 0; max-width: 100%; @@ -3430,6 +3431,7 @@ select { } .diff-line { + position: relative; display: grid; grid-template-columns: 22px 5ch 5ch 2ch minmax(40ch, 1fr); min-height: 20px; @@ -3448,11 +3450,38 @@ select { } .diff-line-select { - align-self: center; - width: 16px; - height: 16px; + position: absolute; + top: 50%; + left: 12px; + width: 18px; + height: 18px; margin: 0; - accent-color: var(--accent); + opacity: 0; + transform: translateY(-50%); +} + +.diff-line-check { + align-self: center; + display: inline-grid; + width: 18px; + height: 18px; + place-items: center; + border-radius: 999px; + color: transparent; + font-family: ui-sans-serif, system-ui, sans-serif; + font-size: 13px; + font-weight: 800; + line-height: 1; +} + +.diff-line-check.selected { + background: var(--accent); + color: var(--accent-contrast); +} + +.diff-line-select:focus-visible + .diff-line-check { + outline: 2px solid var(--accent); + outline-offset: 2px; } .diff-line.added { @@ -3528,7 +3557,13 @@ select { } .pr-inline-comment { + position: sticky; + left: 0; display: grid; + box-sizing: border-box; + width: 100cqw; + min-width: 0; + max-width: 100cqw; gap: 10px; border-top: 1px solid var(--accent); border-bottom: 1px solid var(--border); diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index 845e5bd..c225ca2 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -5905,6 +5905,52 @@ function renderSpeechTranscript(session) { const cursor = session.before.length + (transcript ? session.spacing.length : 0) + transcript.length; input.setSelectionRange(cursor, cursor); input.dispatchEvent(new Event("input", { bubbles: true })); + scrollSpeechCaretIntoView(session, cursor); + window.requestAnimationFrame(() => { + if (input.isConnected) scrollSpeechCaretIntoView(session, cursor); + }); +} + +function scrollSpeechCaretIntoView(session, cursor) { + const input = session?.input; + if (!input?.isConnected) return; + if (!session.after) { + input.scrollTop = input.scrollHeight; + return; + } + + const style = window.getComputedStyle(input); + const mirror = document.createElement("div"); + [ + "boxSizing", "fontFamily", "fontSize", "fontStyle", "fontWeight", "letterSpacing", "lineHeight", + "paddingTop", "paddingRight", "paddingBottom", "paddingLeft", "borderTopWidth", "borderRightWidth", + "borderBottomWidth", "borderLeftWidth", "tabSize", "textIndent", "textTransform", "wordBreak", + "overflowWrap", + ].forEach((property) => { mirror.style[property] = style[property]; }); + mirror.style.position = "fixed"; + mirror.style.top = "0"; + mirror.style.left = "-10000px"; + mirror.style.width = `${input.getBoundingClientRect().width}px`; + mirror.style.height = "auto"; + mirror.style.minHeight = "0"; + mirror.style.maxHeight = "none"; + mirror.style.overflow = "visible"; + mirror.style.visibility = "hidden"; + mirror.style.whiteSpace = "pre-wrap"; + mirror.textContent = input.value.slice(0, cursor); + const marker = document.createElement("span"); + marker.textContent = "\u200b"; + mirror.append(marker); + document.body.append(mirror); + const caretTop = marker.offsetTop; + const lineHeight = Number.parseFloat(style.lineHeight) || marker.getBoundingClientRect().height || 16; + const visibleTop = input.scrollTop; + const visibleBottom = visibleTop + input.clientHeight; + if (caretTop < visibleTop + lineHeight) input.scrollTop = Math.max(0, caretTop - lineHeight); + else if (caretTop + lineHeight > visibleBottom - lineHeight) { + input.scrollTop = Math.min(input.scrollHeight, caretTop + (lineHeight * 2) - input.clientHeight); + } + mirror.remove(); } function finalizeSpeechMode(options = {}) { @@ -5917,6 +5963,10 @@ function finalizeSpeechMode(options = {}) { session.input.value = session.originalValue; session.input.setSelectionRange(session.selectionStart, session.selectionEnd); session.input.dispatchEvent(new Event("input", { bubbles: true })); + session.input.scrollTop = session.originalScrollTop; + window.requestAnimationFrame(() => { + if (session.input.isConnected) session.input.scrollTop = session.originalScrollTop; + }); } else { session.finalTranscript = [session.finalTranscript, session.interimTranscript].filter(Boolean).join(" ").trim(); session.interimTranscript = ""; @@ -6032,6 +6082,7 @@ function startSpeechMode(composer) { const session = { input, originalValue, + originalScrollTop: input.scrollTop, selectionStart, selectionEnd, before: originalValue.slice(0, selectionStart), @@ -6799,12 +6850,17 @@ function renderDiffHunk(hunk, index, options = {}) { const finalSelectedLineIndex = selectedLines.length ? Math.max(...selectedLines.map((line) => Number(line.line_index))) : -1; + const renderLines = (items, offset = 0) => items + .map((line, lineIndex) => renderDiffLine(line, lineIndex + offset, { ...options, hunkIndex: index })) + .join(""); + const beforeSelection = finalSelectedLineIndex >= 0 ? lines.slice(0, finalSelectedLineIndex + 1) : lines; + const afterSelection = finalSelectedLineIndex >= 0 ? lines.slice(finalSelectedLineIndex + 1) : []; return `
${escapeHtml(hunk.header || "@@")}
-
- ${lines.map((line, lineIndex) => `${renderDiffLine(line, lineIndex, { ...options, hunkIndex: index })}${lineIndex === finalSelectedLineIndex ? renderPullRequestInlineComment(options.pullRequestContext, options.path, index) : ""}`).join("")} -
+
${renderLines(beforeSelection)}
+ ${finalSelectedLineIndex >= 0 ? renderPullRequestInlineComment(options.pullRequestContext, options.path, index) : ""} + ${afterSelection.length ? `
${renderLines(afterSelection, finalSelectedLineIndex + 1)}
` : ""}
`; } @@ -6852,7 +6908,7 @@ function renderDiffLine(line, lineIndex = 0, options = {}) { const container = selectable ? "label" : "div"; return ` <${container} class="diff-line ${escapeAttr(kind)}${selected ? " selected" : ""}${selectable ? " selectable" : ""}"> - ${selectable ? `` : ""} + ${selectable ? `` : ""} ${diffLineNumber(line.old_number)} ${diffLineNumber(line.new_number)} ${escapeHtml(marker)} From 6c1019296c4f6e8dc799029a87bdba9872784466 Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Sun, 9 Aug 2026 19:10:51 +0700 Subject: [PATCH 04/12] Keep PR diff comments persistent --- bin/remote-ui-smoke | 112 +++++++++++++++++- lib/hq/remote_ui/assets/app.css | 4 + lib/hq/remote_ui/assets/app.js | 198 +++++++++++++++++++++++++------- 3 files changed, 266 insertions(+), 48 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index 766d98c..5579e20 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -1729,6 +1729,8 @@ def write_smoke_script(path) diff.files[0].hunks[0].lines = Array.from({ length: 101 }, (_, index) => ({ kind: "context", old_number: index + 1, new_number: index + 1, content: `line ${index + 1}`, })); + state.renderedViewHtml = ""; + render(); selectPullRequestDiffLine({ dataset: { agentKey: key, pullRequestId, @@ -1783,6 +1785,74 @@ def write_smoke_script(path) if (!diffErrorState.includes("Diff unavailable") || !diffErrorState.includes("stale fixture diff")) { throw new Error(`PR diff errors are not visible: ${JSON.stringify(diffErrorState)}`); } + const largeDiffSelectionContract = await prContextPage.evaluate(async (key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; + const originalLines = diff.files[0].hunks[0].lines; + diff.files[0].hunks[0].lines = Array.from({ length: 1_500 }, (_, index) => ({ + kind: "context", old_number: index + 1, new_number: index + 1, content: `large diff line ${index + 1}`, + })); + state.renderedViewHtml = ""; + const fullRenderStartedAt = performance.now(); + render(); + const fullRenderSyncMs = performance.now() - fullRenderStartedAt; + await new Promise((resolve) => requestAnimationFrame(resolve)); + const fullRenderMs = performance.now() - fullRenderStartedAt; + + const viewerBefore = document.querySelector("[data-pr-diff-viewer]"); + const target = document.querySelectorAll("[data-select-pr-diff-line]")[900]; + target.scrollIntoView({ block: "center", inline: "nearest" }); + await new Promise((resolve) => requestAnimationFrame(resolve)); + const scrollBefore = window.scrollY; + const lineTopBefore = target.closest(".diff-line").getBoundingClientRect().top; + const startedAt = performance.now(); + target.click(); + const selectionSyncMs = performance.now() - startedAt; + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + const durationMs = performance.now() - startedAt; + const result = { + fullRenderMs, + fullRenderSyncMs, + durationMs, + selectionSyncMs, + viewerPersistent: viewerBefore === document.querySelector("[data-pr-diff-viewer]"), + scrollDelta: Math.abs(window.scrollY - scrollBefore), + lineDelta: Math.abs(target.closest(".diff-line").getBoundingClientRect().top - lineTopBefore), + formVisible: Boolean(document.querySelector("[data-pr-inline-comment]")), + }; + + const pollViewer = document.querySelector("[data-pr-diff-viewer]"); + const pollComment = document.querySelector("[data-pr-context-comment]"); + const pollScrollContainer = document.querySelector(".pr-diff-detail"); + const pollScrollTop = window.scrollY; + const pollLineTop = target.closest(".diff-line").getBoundingClientRect().top; + render({ preserveLiveEditor: true, preservePollContent: true }); + await new Promise((resolve) => requestAnimationFrame(resolve)); + result.pollViewerPersistent = pollViewer === document.querySelector("[data-pr-diff-viewer]"); + result.pollCommentPersistent = pollComment === document.querySelector("[data-pr-context-comment]"); + result.pollContainerReplaced = pollScrollContainer !== document.querySelector(".pr-diff-detail"); + result.pollScrollDelta = Math.abs(window.scrollY - pollScrollTop); + result.pollLineDelta = Math.abs(target.closest(".diff-line").getBoundingClientRect().top - pollLineTop); + + const selection = pullRequestDiffSelection(key, pullRequestId, diff.snapshot_id); + selection.anchor = null; + selection.lines = []; + selection.comment = ""; + diff.files[0].hunks[0].lines = originalLines; + state.renderedViewHtml = ""; + render(); + return result; + }, prContextAgentKey); + if (!largeDiffSelectionContract.viewerPersistent || !largeDiffSelectionContract.formVisible || + !largeDiffSelectionContract.pollViewerPersistent || !largeDiffSelectionContract.pollCommentPersistent || + !largeDiffSelectionContract.pollContainerReplaced || + largeDiffSelectionContract.scrollDelta > 1 || largeDiffSelectionContract.lineDelta > 1 || + largeDiffSelectionContract.pollScrollDelta > 1 || largeDiffSelectionContract.pollLineDelta > 1 || + largeDiffSelectionContract.durationMs > 500) { + throw new Error(`Large PR selection did not preserve the diff, form, or scroll: ${JSON.stringify(largeDiffSelectionContract)}`); + } + console.log(`PR rendering (1,500 lines): full ${largeDiffSelectionContract.fullRenderSyncMs.toFixed(1)}ms sync / selection ${largeDiffSelectionContract.selectionSyncMs.toFixed(1)}ms sync`); await prContextPage.evaluate((key) => { const route = parseRoute(); const pullRequestId = selectedPullRequestId(key, route.pullRequestId); @@ -1797,11 +1867,20 @@ def write_smoke_script(path) await firstLine.click(); await secondLine.click({ modifiers: ["Shift"] }); await prContextPage.waitForSelector("[data-pr-inline-comment]", { state: "visible", timeout: 10_000 }); - const selectedRange = await prContextPage.evaluate(() => ({ - selected: document.querySelectorAll("[data-select-pr-diff-line]:checked").length, - selectionText: document.querySelector("[data-pr-inline-comment]")?.innerText, - overflow: document.documentElement.scrollWidth > document.documentElement.clientWidth, - })); + await prContextPage.waitForFunction(() => document.querySelectorAll("[data-select-pr-diff-line]:checked").length === 2); + const selectedRange = await prContextPage.evaluate((key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; + const selection = pullRequestDiffSelection(key, pullRequestId, diff.snapshot_id); + return { + selected: document.querySelectorAll("[data-select-pr-diff-line]:checked").length, + selectedIndexes: selection.lines.map((line) => line.line_index), + anchorIndex: selection.anchor?.line_index, + selectionText: document.querySelector("[data-pr-inline-comment]")?.innerText, + overflow: document.documentElement.scrollWidth > document.documentElement.clientWidth, + }; + }, prContextAgentKey); if (selectedRange.selected !== 2 || !selectedRange.selectionText?.includes("Commenting on") || selectedRange.overflow) { throw new Error(`PR line range selection is not usable on mobile: ${JSON.stringify(selectedRange)}`); } @@ -1843,6 +1922,29 @@ def write_smoke_script(path) await prContextPage.setViewportSize({ width: 390, height: 844 }); await prContextPage.evaluate(() => document.querySelector("#growl")?.classList.add("hidden")); if (captureDir) await prContextPage.screenshot({ path: path.join(captureDir, "pr-context-selected-mobile.png"), fullPage: true }); + const cancelContract = await prContextPage.evaluate(() => { + const viewer = document.querySelector("[data-pr-diff-viewer]"); + const selectedLine = document.querySelector(".diff-line.selected"); + const lineTop = selectedLine?.getBoundingClientRect().top; + const scrollTop = window.scrollY; + document.querySelector("[data-clear-pr-diff-selection]").click(); + return { + viewerPersistent: viewer === document.querySelector("[data-pr-diff-viewer]"), + formRemoved: !document.querySelector("[data-pr-inline-comment]"), + selectedRows: document.querySelectorAll(".diff-line.selected").length, + checkedLines: document.querySelectorAll("[data-select-pr-diff-line]:checked").length, + selectedMarks: document.querySelectorAll(".diff-line-check.selected").length, + scrollDelta: Math.abs(window.scrollY - scrollTop), + lineDelta: Math.abs((selectedLine?.getBoundingClientRect().top || 0) - (lineTop || 0)), + }; + }); + if (!cancelContract.viewerPersistent || !cancelContract.formRemoved || cancelContract.selectedRows || + cancelContract.checkedLines || cancelContract.selectedMarks || cancelContract.scrollDelta > 1 || cancelContract.lineDelta > 1) { + throw new Error(`Cancel did not clear the persistent PR comment UI in place: ${JSON.stringify(cancelContract)}`); + } + await firstLine.click(); + await secondLine.click({ modifiers: ["Shift"] }); + await prContextPage.waitForFunction(() => document.querySelectorAll("[data-select-pr-diff-line]:checked").length === 2); await prContextPage.keyboard.press("Escape"); if (await prContextPage.locator("[data-select-pr-diff-line]:checked").count()) { const escapeState = await prContextPage.evaluate((key) => ({ diff --git a/lib/hq/remote_ui/assets/app.css b/lib/hq/remote_ui/assets/app.css index b7cd53a..811b8e5 100644 --- a/lib/hq/remote_ui/assets/app.css +++ b/lib/hq/remote_ui/assets/app.css @@ -3539,6 +3539,10 @@ select { border-color: var(--accent); } +.pr-context-selection-placeholder { + visibility: hidden; +} + .pr-context-selection-copy, .pr-context-selection-actions { display: flex; diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index c225ca2..21b3fdd 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -2374,6 +2374,10 @@ function replaceView(html) { } const preserveActiveSpeechEditor = sameRoute && Boolean(state.speechRecognition); + const preservePullRequestDiff = sameRoute && Boolean(state.renderedViewHtml) && parseRoute().type === "agentPullRequests"; + const preservedPollContent = sameRoute && (state.preservePollContentDuringRender || preservePullRequestDiff) + ? preservedPollContentByKey(els.view) + : new Map(); const liveEditorPlan = sameRoute && (state.preserveLiveEditorDuringRender || preserveActiveSpeechEditor) ? liveEditorRefreshPlan(html) : null; @@ -2381,6 +2385,7 @@ function replaceView(html) { const reconciled = liveEditorPlan ? reconcileViewAroundEditor(liveEditorPlan) : false; if (!reconciled && state.speechRecognition) stopSpeechMode({ immediate: true }); if (!reconciled) replaceViewContent(html); + transplantPreservedPollContent(els.view, preservedPollContent); state.renderedRouteKey = routeKey; state.renderedViewHtml = html; syncMarkdownHeadingAnchors(); @@ -2431,7 +2436,7 @@ function transplantPreservedPollContent(incomingRoot, preserved) { if (!preserved?.size) return; pollContentElements(incomingRoot).forEach((incoming) => { const current = preserved.get(pollContentKey(incoming)); - if (current) incoming.replaceWith(current); + if (current && current !== incoming) incoming.replaceWith(current); }); } @@ -5363,36 +5368,47 @@ function renderPullRequestDiffDetail(agent, item, diff, options = {}) { ${escapeHtml([diff.head_sha ? `head ${shortSha(diff.head_sha)}` : "", diff.fetched_at ? `fetched ${timeShort(diff.fetched_at)}` : ""].filter(Boolean).join(" / "))} - ${renderPullRequestLineSelection(agent, item, selection)} -
+
+ ${renderPullRequestLineSelection(agent, item, selection)} +
+
${files.length ? files.map((file, index) => renderProjectDiffFile(file, index, { openAll: expandAll, - pullRequestContext: { - agentKey: agent.key, - pullRequestId: item.id, - snapshotId: diff.snapshot_id || "", - repository: diff.repository || item.repository, - number: diff.number || item.number, - selection, - selectedLines: selection.lines, - selectionLocked: state.pendingPullRequestCommentKeys.has(agent.key), - }, + pullRequestContext: pullRequestDiffRenderContext(agent, item, diff, selection), })).join("") : emptyState("No file changes", "This pull request diff snapshot has no textual file changes.")}
`; } +function pullRequestDiffRenderContext(agent, item, diff, selection) { + return { + agentKey: agent.key, + pullRequestId: item.id, + snapshotId: diff.snapshot_id || "", + repository: diff.repository || item.repository, + number: diff.number || item.number, + selection, + selectedLines: selection.lines, + selectionLocked: state.pendingPullRequestCommentKeys.has(agent.key), + }; +} + function renderPullRequestLineSelection(agent, item, selection) { const count = selection.lines.length; - if (count) return ""; const pending = pendingPullRequestContextsFor(agent.key).length; return ` -
+
${iconSvg("botMessageSquare")} ${pending ? `${pending} comment section${pending === 1 ? "" : "s"} ready` : "Comment on the diff"}${pending ? "Select another range, or review the combined comment." : "Select a line, then Shift+select another line for a contiguous range."} - ${pending ? `` : ""} + ${pending ? `` : ""}
`; } @@ -6850,17 +6866,14 @@ function renderDiffHunk(hunk, index, options = {}) { const finalSelectedLineIndex = selectedLines.length ? Math.max(...selectedLines.map((line) => Number(line.line_index))) : -1; - const renderLines = (items, offset = 0) => items - .map((line, lineIndex) => renderDiffLine(line, lineIndex + offset, { ...options, hunkIndex: index })) - .join(""); - const beforeSelection = finalSelectedLineIndex >= 0 ? lines.slice(0, finalSelectedLineIndex + 1) : lines; - const afterSelection = finalSelectedLineIndex >= 0 ? lines.slice(finalSelectedLineIndex + 1) : []; + const renderedLines = lines.map((line, lineIndex) => ` + ${renderDiffLine(line, lineIndex, { ...options, hunkIndex: index })} + ${lineIndex === finalSelectedLineIndex ? renderPullRequestInlineComment(options.pullRequestContext, options.path, index) : ""} + `).join(""); return `
${escapeHtml(hunk.header || "@@")}
-
${renderLines(beforeSelection)}
- ${finalSelectedLineIndex >= 0 ? renderPullRequestInlineComment(options.pullRequestContext, options.path, index) : ""} - ${afterSelection.length ? `
${renderLines(afterSelection, finalSelectedLineIndex + 1)}
` : ""} +
${renderedLines}
`; } @@ -7002,10 +7015,114 @@ function selectPullRequestDiffLine(target, extendRange = false) { selected.lines = [line]; selected.comment = ""; } - render(); - if (selected.lines.length) { - window.requestAnimationFrame(() => document.querySelector("[data-pr-context-comment]")?.focus({ preventScroll: true })); + syncPullRequestDiffSelection(agentKey, pullRequestId, { focusComment: selected.lines.length > 0, scrollTarget: target }); +} + +function syncPullRequestDiffSelection(agentKey, pullRequestId, options = {}) { + const agent = findAgent(agentKey); + const item = arrayValue(state.pullRequests[agentKey]?.items).find((candidate) => candidate.id === pullRequestId); + const diff = state.pullRequestDiffs[pullRequestDiffKey(agentKey, pullRequestId)]; + if (!agent || !item || !diff || diff.error) return false; + + const selection = pullRequestDiffSelection(agentKey, pullRequestId, diff.snapshot_id || ""); + const viewer = document.querySelector( + `[data-pr-diff-viewer][data-agent-key="${CSS.escape(agentKey)}"][data-pull-request-id="${CSS.escape(pullRequestId)}"]` + ); + if (!viewer) return false; + + const scrollSnapshot = capturePullRequestScrollPosition(options.scrollTarget instanceof Element ? options.scrollTarget : viewer); + const selectedKeys = new Set(selection.lines.map(pullRequestLineKey)); + const controls = Array.from(viewer.querySelectorAll("[data-select-pr-diff-line]")); + const selectionLocked = state.pendingPullRequestCommentKeys.has(agentKey); + syncPullRequestDiffLineControls(controls, selectedKeys, selectionLocked); + + viewer.querySelectorAll("[data-pr-inline-comment]").forEach((comment) => comment.remove()); + if (selection.lines.length) { + const finalLine = selection.lines.at(-1); + const finalControl = controls.find((control) => ( + pullRequestLineKey(pullRequestLineDescriptor(control)) === pullRequestLineKey(finalLine) + )); + finalControl?.closest(".diff-line")?.insertAdjacentHTML( + "afterend", + renderPullRequestInlineComment( + pullRequestDiffRenderContext(agent, item, diff, selection), + finalLine.path, + finalLine.hunk_index + ) + ); + } + + const selectionSlot = document.querySelector("[data-pr-context-selection-slot]"); + if (selectionSlot) selectionSlot.innerHTML = renderPullRequestLineSelection(agent, item, selection); + restorePullRequestScrollPosition(scrollSnapshot); + window.requestAnimationFrame(() => { + // A native checkbox inside a label can finish its activation after the delegated click handler. + // Reassert the state model once without rebuilding the diff. + syncPullRequestDiffLineControls(controls, selectedKeys, selectionLocked); + if (options.focusComment && selection.lines.length) { + document.querySelector("[data-pr-context-comment]")?.focus({ preventScroll: true }); + } + restorePullRequestScrollPosition(scrollSnapshot); + }); + return true; +} + +function syncPullRequestDiffLineControls(controls, selectedKeys, selectionLocked = false) { + controls.forEach((control) => { + const key = pullRequestLineKey(pullRequestLineDescriptor(control)); + const checked = selectedKeys.has(key); + control.checked = checked; + control.disabled = selectionLocked; + control.closest(".diff-line")?.classList.toggle("selected", checked); + const mark = control.nextElementSibling; + mark?.classList.toggle("selected", checked); + if (mark) mark.textContent = checked ? "✓" : ""; + }); +} + +function pullRequestLineDescriptor(control) { + return { + path: control.dataset.reviewLinePath, + hunk_index: Number(control.dataset.reviewLineHunk), + line_index: Number(control.dataset.reviewLineIndex), + }; +} + +function capturePullRequestScrollPosition(target) { + const anchor = target.closest?.(".diff-line") || null; + const containers = []; + let current = target; + while (current && current !== document.body) { + if (current.matches?.("[data-preserve-scroll]")) { + containers.push({ element: current, top: current.scrollTop, left: current.scrollLeft }); + } + current = current.parentElement; } + return { + anchor, + anchorTop: anchor?.getBoundingClientRect().top, + containers, + page: { top: window.scrollY, left: window.scrollX }, + }; +} + +function restorePullRequestScrollPosition(snapshot) { + if (!snapshot) return; + snapshot.containers.forEach(({ element, top, left }) => { + if (!element.isConnected) return; + element.scrollTop = top; + element.scrollLeft = left; + }); + window.scrollTo(snapshot.page.left, snapshot.page.top); + if (!snapshot.anchor?.isConnected || !Number.isFinite(snapshot.anchorTop)) return; + + const anchorDelta = snapshot.anchor.getBoundingClientRect().top - snapshot.anchorTop; + if (Math.abs(anchorDelta) < 0.5) return; + const verticalContainer = snapshot.containers.find(({ element }) => ( + element.scrollHeight > element.clientHeight && ["auto", "scroll"].includes(getComputedStyle(element).overflowY) + )); + if (verticalContainer) verticalContainer.element.scrollTop += anchorDelta; + else window.scrollBy(0, anchorDelta); } function pendingPullRequestContextsFor(agentKey) { @@ -7061,7 +7178,7 @@ function attachSelectedPullRequestLines(agentKey, pullRequestId) { selection.anchor = null; selection.lines = []; selection.comment = ""; - document.querySelectorAll("[data-select-pr-diff-line]").forEach((control) => { control.checked = false; }); + syncPullRequestDiffSelection(agentKey, pullRequestId); render(); showGrowl("Comment section added", "done"); } @@ -7094,6 +7211,7 @@ function sendSelectedPullRequestComment(agentKey, pullRequestId) { pendingPromptMessageBlock(agentKey, prompt, [], [context]) ); state.pendingPullRequestCommentKeys.add(agentKey); + syncPullRequestDiffSelection(agentKey, pullRequestId); render(); mutate(async () => { await apiPost(`/agents/${encodeURIComponent(agentKey)}/messages`, { @@ -7110,6 +7228,7 @@ function sendSelectedPullRequestComment(agentKey, pullRequestId) { removePendingConversationMessage(agentKey, pendingMessageId, { render: false }); }).finally(() => { state.pendingPullRequestCommentKeys.delete(agentKey); + syncPullRequestDiffSelection(agentKey, pullRequestId); removePendingConversationMessage(agentKey, pendingMessageId); }); } @@ -7124,14 +7243,17 @@ function clearActivePullRequestDiffSelection() { const route = parseRoute(); if (route.type !== "agentPullRequests") return false; const pullRequestId = selectedPullRequestId(route.key, route.pullRequestId); - const diff = state.pullRequestDiffs[pullRequestDiffKey(route.key, pullRequestId)]; - const selection = pullRequestDiffSelection(route.key, pullRequestId, diff?.snapshot_id || ""); + return clearPullRequestDiffSelection(route.key, pullRequestId); +} + +function clearPullRequestDiffSelection(agentKey, pullRequestId) { + const diff = state.pullRequestDiffs[pullRequestDiffKey(agentKey, pullRequestId)]; + const selection = pullRequestDiffSelection(agentKey, pullRequestId, diff?.snapshot_id || ""); if (!selection.lines.length) return false; selection.anchor = null; selection.lines = []; selection.comment = ""; - document.querySelectorAll("[data-select-pr-diff-line]").forEach((control) => { control.checked = false; }); - render(); + syncPullRequestDiffSelection(agentKey, pullRequestId); return true; } @@ -12144,20 +12266,10 @@ els.view.addEventListener("click", (event) => { } const clearPrDiffSelection = event.target.closest("[data-clear-pr-diff-selection]"); if (clearPrDiffSelection) { - const diff = state.pullRequestDiffs[pullRequestDiffKey( + clearPullRequestDiffSelection( clearPrDiffSelection.dataset.agentKey, clearPrDiffSelection.dataset.pullRequestId - )]; - const selection = pullRequestDiffSelection( - clearPrDiffSelection.dataset.agentKey, - clearPrDiffSelection.dataset.pullRequestId, - diff?.snapshot_id || "" ); - selection.anchor = null; - selection.lines = []; - selection.comment = ""; - document.querySelectorAll("[data-select-pr-diff-line]").forEach((control) => { control.checked = false; }); - render(); return; } const removePrContext = event.target.closest("[data-remove-pr-context]"); From 1e38e61c477e406242de811931dc05c0d0269eef Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Sun, 9 Aug 2026 19:53:27 +0700 Subject: [PATCH 05/12] Skip unchanged PR diff rendering --- bin/remote-ui-smoke | 92 ++++++++++++++++++- lib/hq/remote_ui/assets/app.js | 159 ++++++++++++++++++++++++++------- test/remote_server_test.rb | 2 +- 3 files changed, 217 insertions(+), 36 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index 5579e20..d971a4f 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -1790,6 +1790,8 @@ def write_smoke_script(path) const pullRequestId = selectedPullRequestId(key, route.pullRequestId); const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; const originalLines = diff.files[0].hunks[0].lines; + const originalSnapshotId = diff.snapshot_id; + const originalExpandAll = state.prDiffExpandAll[key]; diff.files[0].hunks[0].lines = Array.from({ length: 1_500 }, (_, index) => ({ kind: "context", old_number: index + 1, new_number: index + 1, content: `large diff line ${index + 1}`, })); @@ -1827,18 +1829,89 @@ def write_smoke_script(path) const pollScrollContainer = document.querySelector(".pr-diff-detail"); const pollScrollTop = window.scrollY; const pollLineTop = target.closest(".diff-line").getBoundingClientRect().top; + const originalRenderDiffLine = renderDiffLine; + let pollDiffLineRenderCount = 0; + renderDiffLine = (...args) => { + pollDiffLineRenderCount += 1; + return originalRenderDiffLine(...args); + }; + const pollRenderStartedAt = performance.now(); render({ preserveLiveEditor: true, preservePollContent: true }); + result.pollRenderSyncMs = performance.now() - pollRenderStartedAt; + result.pollDiffLineRenderCount = pollDiffLineRenderCount; + renderDiffLine = originalRenderDiffLine; await new Promise((resolve) => requestAnimationFrame(resolve)); result.pollViewerPersistent = pollViewer === document.querySelector("[data-pr-diff-viewer]"); result.pollCommentPersistent = pollComment === document.querySelector("[data-pr-context-comment]"); + result.pollFocusPersistent = document.activeElement === pollComment; result.pollContainerReplaced = pollScrollContainer !== document.querySelector(".pr-diff-detail"); result.pollScrollDelta = Math.abs(window.scrollY - pollScrollTop); result.pollLineDelta = Math.abs(target.closest(".diff-line").getBoundingClientRect().top - pollLineTop); + pollComment.value = "Live comment text must not invalidate the diff shell."; + pollComment.dispatchEvent(new Event("input", { bubbles: true })); + let steadyPollDiffLineRenderCount = 0; + renderDiffLine = (...args) => { + steadyPollDiffLineRenderCount += 1; + return originalRenderDiffLine(...args); + }; + const steadyPollStartedAt = performance.now(); + render({ preserveLiveEditor: true, preservePollContent: true }); + result.steadyPollRenderSyncMs = performance.now() - steadyPollStartedAt; + renderDiffLine = originalRenderDiffLine; + result.steadyPollDiffLineRenderCount = steadyPollDiffLineRenderCount; + result.steadyPollComment = document.querySelector("[data-pr-context-comment]")?.value; + result.steadyPollViewerPersistent = pollViewer === document.querySelector("[data-pr-diff-viewer]"); + + const item = state.pullRequests[key].items.find((candidate) => candidate.id === pullRequestId); + const originalTitle = item.title; + item.title = `${originalTitle} updated`; + let forcedRenderDiffLineCount = 0; + renderDiffLine = (...args) => { + forcedRenderDiffLineCount += 1; + return originalRenderDiffLine(...args); + }; + render({ preserveLiveEditor: true, preservePollContent: false }); + renderDiffLine = originalRenderDiffLine; + result.forcedRenderDiffLineCount = forcedRenderDiffLineCount; + result.forcedViewerPersistent = pollViewer === document.querySelector("[data-pr-diff-viewer]"); + result.forcedCommentPersistent = pollComment === document.querySelector("[data-pr-context-comment]"); + result.forcedTitleUpdated = document.querySelector(".pr-diff-title-meta strong")?.textContent === item.title; + item.title = originalTitle; + + let changedSnapshotRenderCount = 0; + renderDiffLine = (...args) => { + changedSnapshotRenderCount += 1; + return originalRenderDiffLine(...args); + }; + diff.snapshot_id = `${originalSnapshotId}-changed`; + render({ preserveLiveEditor: true, preservePollContent: true }); + renderDiffLine = originalRenderDiffLine; + result.changedSnapshotRenderCount = changedSnapshotRenderCount; + result.changedSnapshotViewerReplaced = pollViewer !== document.querySelector("[data-pr-diff-viewer]"); + + const changedSnapshotViewer = document.querySelector("[data-pr-diff-viewer]"); + diff.snapshot_id = originalSnapshotId; + let collapsedRenderCount = 0; + renderDiffLine = (...args) => { + collapsedRenderCount += 1; + return originalRenderDiffLine(...args); + }; + document.querySelector(`[data-toggle-pr-diff-expand-all="${CSS.escape(key)}"]`).click(); + renderDiffLine = originalRenderDiffLine; + result.collapsedRenderCount = collapsedRenderCount; + result.collapsedViewerReplaced = changedSnapshotViewer !== document.querySelector("[data-pr-diff-viewer]"); + result.collapsedFilesClosed = Array.from(document.querySelectorAll("[data-pr-diff-viewer] .diff-file")) + .every((detail) => !detail.open); const selection = pullRequestDiffSelection(key, pullRequestId, diff.snapshot_id); selection.anchor = null; selection.lines = []; selection.comment = ""; + if (originalExpandAll === undefined) delete state.prDiffExpandAll[key]; + else state.prDiffExpandAll[key] = originalExpandAll; + document.querySelectorAll(".pr-diff-detail .diff-file").forEach((detail) => { + detail.open = originalExpandAll !== false; + }); diff.files[0].hunks[0].lines = originalLines; state.renderedViewHtml = ""; render(); @@ -1846,13 +1919,28 @@ def write_smoke_script(path) }, prContextAgentKey); if (!largeDiffSelectionContract.viewerPersistent || !largeDiffSelectionContract.formVisible || !largeDiffSelectionContract.pollViewerPersistent || !largeDiffSelectionContract.pollCommentPersistent || - !largeDiffSelectionContract.pollContainerReplaced || + !largeDiffSelectionContract.pollFocusPersistent || + largeDiffSelectionContract.pollContainerReplaced || + largeDiffSelectionContract.pollDiffLineRenderCount !== 0 || + !largeDiffSelectionContract.steadyPollViewerPersistent || + largeDiffSelectionContract.steadyPollDiffLineRenderCount !== 0 || + largeDiffSelectionContract.steadyPollComment !== "Live comment text must not invalidate the diff shell." || + largeDiffSelectionContract.steadyPollRenderSyncMs >= largeDiffSelectionContract.fullRenderSyncMs * 0.5 || + largeDiffSelectionContract.forcedRenderDiffLineCount !== 0 || + !largeDiffSelectionContract.forcedViewerPersistent || + !largeDiffSelectionContract.forcedCommentPersistent || + !largeDiffSelectionContract.forcedTitleUpdated || + largeDiffSelectionContract.changedSnapshotRenderCount !== 1_500 || + !largeDiffSelectionContract.changedSnapshotViewerReplaced || + largeDiffSelectionContract.collapsedRenderCount !== 1_500 || + !largeDiffSelectionContract.collapsedViewerReplaced || + !largeDiffSelectionContract.collapsedFilesClosed || largeDiffSelectionContract.scrollDelta > 1 || largeDiffSelectionContract.lineDelta > 1 || largeDiffSelectionContract.pollScrollDelta > 1 || largeDiffSelectionContract.pollLineDelta > 1 || largeDiffSelectionContract.durationMs > 500) { throw new Error(`Large PR selection did not preserve the diff, form, or scroll: ${JSON.stringify(largeDiffSelectionContract)}`); } - console.log(`PR rendering (1,500 lines): full ${largeDiffSelectionContract.fullRenderSyncMs.toFixed(1)}ms sync / selection ${largeDiffSelectionContract.selectionSyncMs.toFixed(1)}ms sync`); + console.log(`PR rendering (1,500 lines): full ${largeDiffSelectionContract.fullRenderSyncMs.toFixed(1)}ms sync / selection ${largeDiffSelectionContract.selectionSyncMs.toFixed(1)}ms sync / first poll ${largeDiffSelectionContract.pollRenderSyncMs.toFixed(1)}ms sync / steady poll ${largeDiffSelectionContract.steadyPollRenderSyncMs.toFixed(1)}ms sync`); await prContextPage.evaluate((key) => { const route = parseRoute(); const pullRequestId = selectedPullRequestId(key, route.pullRequestId); diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index 21b3fdd..a939896 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -2381,7 +2381,7 @@ function replaceView(html) { const liveEditorPlan = sameRoute && (state.preserveLiveEditorDuringRender || preserveActiveSpeechEditor) ? liveEditorRefreshPlan(html) : null; - const snapshot = sameRoute ? captureViewState() : null; + const snapshot = sameRoute ? captureViewState(preservedPollContent) : null; const reconciled = liveEditorPlan ? reconcileViewAroundEditor(liveEditorPlan) : false; if (!reconciled && state.speechRecognition) stopSpeechMode({ immediate: true }); if (!reconciled) replaceViewContent(html); @@ -2390,7 +2390,7 @@ function replaceView(html) { state.renderedViewHtml = html; syncMarkdownHeadingAnchors(); restoreFormDrafts(); - restoreViewState(snapshot); + restoreViewState(snapshot, preservedPollContent); syncPendingForms(); syncViewControls(); syncFullScreenComposerModal(); @@ -2459,8 +2459,12 @@ function editorFullScreen(editor) { } function reconcileViewAroundEditor(plan) { - const currentChain = editorAncestorChain(plan.editor, els.view); - const incomingChain = editorAncestorChain(plan.incomingEditor, plan.incomingRoot); + return reconcileNodesAroundAnchor(plan.editor, plan.incomingEditor, els.view, plan.incomingRoot); +} + +function reconcileNodesAroundAnchor(currentAnchor, incomingAnchor, currentRoot, incomingRoot) { + const currentChain = elementAncestorChain(currentAnchor, currentRoot); + const incomingChain = elementAncestorChain(incomingAnchor, incomingRoot); if (!currentChain || !incomingChain || currentChain.length !== incomingChain.length) return false; if (currentChain.some((node, index) => index > 0 && index < currentChain.length - 1 && node.tagName !== incomingChain[index].tagName)) return false; @@ -2469,15 +2473,15 @@ function reconcileViewAroundEditor(plan) { const incomingParent = incomingChain[index]; const currentBranch = currentChain[index - 1]; const incomingBranch = incomingChain[index - 1]; - if (currentParent !== els.view) syncElementAttributes(currentParent, incomingParent); - replaceSiblingsAroundEditor(currentParent, currentBranch, incomingParent, incomingBranch); + if (currentParent !== currentRoot) syncElementAttributes(currentParent, incomingParent); + replaceSiblingsAroundAnchor(currentParent, currentBranch, incomingParent, incomingBranch); } return true; } -function editorAncestorChain(editor, root) { - const chain = [editor]; - let current = editor; +function elementAncestorChain(element, root) { + const chain = [element]; + let current = element; while (current && current !== root) { current = current.parentNode; if (current) chain.push(current); @@ -2494,12 +2498,20 @@ function syncElementAttributes(current, incoming) { }); } -function replaceSiblingsAroundEditor(currentParent, currentBranch, incomingParent, incomingBranch) { +function replaceSiblingsAroundAnchor(currentParent, currentBranch, incomingParent, incomingBranch) { const preservedPollContent = state.preservePollContentDuringRender ? preservedPollContentByKey(currentParent) : new Map(); + const retainedBranches = reconcilePullRequestDiffSiblingBranch( + currentParent, + currentBranch, + incomingParent, + incomingBranch, + preservedPollContent + ); + const retainedNodes = new Set(retainedBranches.values()); Array.from(currentParent.childNodes).forEach((node) => { - if (node !== currentBranch) node.remove(); + if (node !== currentBranch && !retainedNodes.has(node)) node.remove(); }); let afterBranch = false; @@ -2508,23 +2520,62 @@ function replaceSiblingsAroundEditor(currentParent, currentBranch, incomingParen afterBranch = true; return; } - const clone = node.cloneNode(true); - transplantPreservedPollContent(clone, preservedPollContent); - if (afterBranch) currentParent.appendChild(clone); - else currentParent.insertBefore(clone, currentBranch); + const replacement = retainedBranches.get(node) || node.cloneNode(true); + if (!retainedBranches.has(node)) transplantPreservedPollContent(replacement, preservedPollContent); + if (afterBranch) currentParent.appendChild(replacement); + else currentParent.insertBefore(replacement, currentBranch); + }); +} + +function reconcilePullRequestDiffSiblingBranch(currentParent, currentBranch, incomingParent, incomingBranch, preserved) { + if (!preserved.size) return new Map(); + + const currentBranchesByKey = new Map(); + Array.from(currentParent.children).forEach((branch) => { + if (branch === currentBranch) return; + pollContentElements(branch).filter((element) => element.matches("[data-pr-diff-viewer]")).forEach((element) => { + const key = pollContentKey(element); + if (key && preserved.get(key) === element) currentBranchesByKey.set(key, { branch, element }); + }); }); + + const retained = new Map(); + Array.from(incomingParent.children).forEach((incomingBranchCandidate) => { + if (incomingBranchCandidate === incomingBranch) return; + const incomingElement = pollContentElements(incomingBranchCandidate) + .filter((element) => element.matches("[data-pr-diff-viewer]")) + .find((element) => currentBranchesByKey.has(pollContentKey(element))); + if (!incomingElement) return; + + const current = currentBranchesByKey.get(pollContentKey(incomingElement)); + if (current.branch.tagName !== incomingBranchCandidate.tagName) return; + if (!reconcileNodesAroundAnchor( + current.element, + incomingElement, + current.branch, + incomingBranchCandidate + )) return; + syncElementAttributes(current.branch, incomingBranchCandidate); + retained.set(incomingBranchCandidate, current.branch); + }); + return retained; } function routeStateKey(route) { return REMOTE_HELPERS.routeStateKey(route); } -function captureViewState() { +function captureViewState(preserved = new Map()) { const active = document.activeElement; + const skippedRoots = preservedPullRequestDiffRoots(preserved); + const elements = viewStateElements(skippedRoots); + const preservedActive = Array.from(skippedRoots).some((root) => root.contains(active)) ? active : null; const snapshot = { activeKey: null, activeFocusKey: null, activeSelection: null, + preservedActive, + preservedActiveSelection: preservedActive ? textSelectionFor(preservedActive) : null, controls: {}, details: {}, openElements: {}, @@ -2535,7 +2586,7 @@ function captureViewState() { scrollContainers: {}, }; - els.view.querySelectorAll("input, textarea, select").forEach((control, index) => { + elements.filter((element) => element.matches("input, textarea, select")).forEach((control, index) => { const key = elementStateKey(control, index); if (control === active) { snapshot.activeKey = key; @@ -2545,19 +2596,19 @@ function captureViewState() { snapshot.controls[key] = controlState(control); }); - els.view.querySelectorAll("[data-preserve-focus]").forEach((element, index) => { + elements.filter((element) => element.matches("[data-preserve-focus]")).forEach((element, index) => { if (element === active) snapshot.activeFocusKey = elementStateKey(element, index); }); - els.view.querySelectorAll("details").forEach((detail, index) => { + elements.filter((element) => element.matches("details")).forEach((detail, index) => { snapshot.details[elementStateKey(detail, index)] = detail.open; }); - els.view.querySelectorAll("[data-preserve-open]").forEach((element, index) => { + elements.filter((element) => element.matches("[data-preserve-open]")).forEach((element, index) => { snapshot.openElements[elementStateKey(element, index)] = !element.classList.contains("hidden"); }); - els.view.querySelectorAll("[data-preserve-scroll]").forEach((element, index) => { + elements.filter((element) => element.matches("[data-preserve-scroll]")).forEach((element, index) => { snapshot.scrollContainers[elementStateKey(element, index)] = { top: element.scrollTop, left: element.scrollLeft, @@ -2567,10 +2618,11 @@ function captureViewState() { return snapshot; } -function restoreViewState(snapshot) { +function restoreViewState(snapshot, preserved = new Map()) { if (!snapshot) return; + const elements = viewStateElements(preservedPullRequestDiffRoots(preserved)); - els.view.querySelectorAll("input, textarea, select").forEach((control, index) => { + elements.filter((element) => element.matches("input, textarea, select")).forEach((control, index) => { const key = elementStateKey(control, index); const stored = snapshot.controls[key]; if (stored) restoreControlState(control, stored); @@ -2579,20 +2631,20 @@ function restoreViewState(snapshot) { restoreTextSelection(control, snapshot.activeSelection); } }); - els.view.querySelectorAll("[data-preserve-focus]").forEach((element, index) => { + elements.filter((element) => element.matches("[data-preserve-focus]")).forEach((element, index) => { if (snapshot.activeFocusKey === elementStateKey(element, index)) { element.focus({ preventScroll: true }); } }); - els.view.querySelectorAll("details").forEach((detail, index) => { + elements.filter((element) => element.matches("details")).forEach((detail, index) => { const key = elementStateKey(detail, index); if (Object.prototype.hasOwnProperty.call(snapshot.details, key)) { detail.open = snapshot.details[key]; } }); - els.view.querySelectorAll("[data-preserve-open]").forEach((element, index) => { + elements.filter((element) => element.matches("[data-preserve-open]")).forEach((element, index) => { const key = elementStateKey(element, index); if (Object.prototype.hasOwnProperty.call(snapshot.openElements, key)) { const open = snapshot.openElements[key]; @@ -2601,7 +2653,7 @@ function restoreViewState(snapshot) { } }); - els.view.querySelectorAll("[data-preserve-scroll]").forEach((element, index) => { + elements.filter((element) => element.matches("[data-preserve-scroll]")).forEach((element, index) => { const stored = snapshot.scrollContainers[elementStateKey(element, index)]; if (!stored) return; @@ -2609,11 +2661,36 @@ function restoreViewState(snapshot) { element.scrollLeft = stored.left || 0; }); + if (snapshot.preservedActive?.isConnected && document.activeElement !== snapshot.preservedActive) { + snapshot.preservedActive.focus({ preventScroll: true }); + restoreTextSelection(snapshot.preservedActive, snapshot.preservedActiveSelection); + } + restorePageScroll(snapshot.pageScroll); repositionOpenSummaryAttachmentMenus(); restoreSkillAutocompleteAfterRender(); } +function viewStateElements(skippedRoots = new Set()) { + const elements = []; + const walker = document.createTreeWalker(els.view, NodeFilter.SHOW_ELEMENT, { + acceptNode(element) { + if (skippedRoots.has(element)) return NodeFilter.FILTER_REJECT; + return NodeFilter.FILTER_ACCEPT; + }, + }); + let element = walker.nextNode(); + while (element) { + elements.push(element); + element = walker.nextNode(); + } + return elements; +} + +function preservedPullRequestDiffRoots(preserved) { + return new Set(Array.from(preserved.values()).filter((element) => element.matches("[data-pr-diff-viewer]"))); +} + function syncPreservedOpenState(element, open) { if (element.matches("[data-attachment-flyout]")) { document.querySelector("[data-toggle-attachments]")?.setAttribute("aria-expanded", open ? "true" : "false"); @@ -5360,6 +5437,17 @@ function renderPullRequestDiffDetail(agent, item, diff, options = {}) { const files = Array.isArray(diff.files) ? diff.files : []; const summary = `${files.length} ${files.length === 1 ? "file" : "files"} / +${diff.additions || 0} -${diff.deletions || 0}`; const selection = pullRequestDiffSelection(agent.key, item.id, diff.snapshot_id || ""); + const viewerStateKey = `pr-diff-viewer:${agent.key}:${item.id}`; + const viewerVersion = `${diff.snapshot_id || "none"}:${expandAll ? "expanded" : "collapsed"}`; + const viewerBody = reusablePullRequestDiffViewer(agent.key, item.id, viewerStateKey, viewerVersion) + ? "" + : files.length + ? files.map((file, index) => renderProjectDiffFile(file, index, { + openAll: expandAll, + closeAll: !expandAll, + pullRequestContext: pullRequestDiffRenderContext(agent, item, diff, selection), + })).join("") + : emptyState("No file changes", "This pull request diff snapshot has no textual file changes."); return ` ${header}
@@ -5376,16 +5464,21 @@ function renderPullRequestDiffDetail(agent, item, diff, options = {}) { data-agent-key="${escapeAttr(agent.key)}" data-pull-request-id="${escapeAttr(item.id)}" data-preserve-poll-content - data-state-key="pr-diff-viewer:${escapeAttr(agent.key)}:${escapeAttr(item.id)}" - data-poll-content-version="${escapeAttr(`${diff.snapshot_id || "none"}:${expandAll ? "expanded" : "collapsed"}`)}"> - ${files.length ? files.map((file, index) => renderProjectDiffFile(file, index, { - openAll: expandAll, - pullRequestContext: pullRequestDiffRenderContext(agent, item, diff, selection), - })).join("") : emptyState("No file changes", "This pull request diff snapshot has no textual file changes.")} + data-state-key="${escapeAttr(viewerStateKey)}" + data-poll-content-version="${escapeAttr(viewerVersion)}"> + ${viewerBody}
`; } +function reusablePullRequestDiffViewer(agentKey, pullRequestId, stateKey, version) { + if (!state.renderedViewHtml || state.renderedRouteKey !== routeStateKey(parseRoute())) return null; + const viewer = document.querySelector( + `[data-pr-diff-viewer][data-agent-key="${CSS.escape(agentKey)}"][data-pull-request-id="${CSS.escape(pullRequestId)}"]` + ); + return viewer && pollContentKey(viewer) === `${stateKey}:${version}` ? viewer : null; +} + function pullRequestDiffRenderContext(agent, item, diff, selection) { return { agentKey: agent.key, diff --git a/test/remote_server_test.rb b/test/remote_server_test.rb index b398f35..15bf023 100644 --- a/test/remote_server_test.rb +++ b/test/remote_server_test.rb @@ -4959,7 +4959,7 @@ def assert_remote_ui_routes_load_without_auth js[:body].include?("function liveEditorRefreshPlan") && js[:body].include?("function reconcileViewAroundEditor"), "expected polling renders to reconcile around stable Conversation and inquiry forms") - assert(js[:body].include?("replaceSiblingsAroundEditor") && + assert(js[:body].include?("replaceSiblingsAroundAnchor") && js[:body].include?('data-agent-running="${agentIsRunning(agent) ? "true" : "false"}"'), "expected live editor reconciliation to preserve controls without hiding real agent state transitions") assert(js[:body].include?("scrollContainers"), From 76164075778d70dad38027decdef571d8e41a3cb Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Sun, 9 Aug 2026 21:09:48 +0700 Subject: [PATCH 06/12] Persist agent pull request listings --- bin/remote-ui-smoke | 35 ++++++ docs/PROJECT_STATUS.md | 1 + docs/PULL_REQUEST_DIFFS.md | 2 + docs/REMOTE_SERVER.md | 3 +- lib/hq/domain/managed_agent.rb | 7 ++ lib/hq/domain/pull_request_diff.rb | 107 +++++++++++++++++- lib/hq/remote_server.rb | 72 ++++++++++-- lib/hq/remote_ui/assets/app.js | 22 +++- test/remote_server_test.rb | 171 +++++++++++++++++++++++++++-- 9 files changed, 396 insertions(+), 24 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index d971a4f..0c2c86b 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -1713,6 +1713,41 @@ def write_smoke_script(path) await prContextPage.waitForSelector(".agent-pr-diff-shell .empty-state", { state: "visible", timeout: 10_000 }); await prContextPage.click("[data-refresh-pr-diff]"); await prContextPage.waitForSelector("[data-select-pr-diff-line]", { state: "visible", timeout: 10_000 }); + const prOriginMetadata = await prContextPage.evaluate((key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const item = state.pullRequests[key].items.find((candidate) => candidate.id === pullRequestId); + const result = { + itemTitle: item?.title, + navTitle: document.querySelector(".pr-diff-nav-item.active strong")?.textContent, + detailTitle: document.querySelector(".pr-diff-title-meta strong")?.textContent, + states: [], + }; + [ + { state: "open", draft: false, merged: false, expected: "Open" }, + { state: "open", draft: true, merged: false, expected: "Draft" }, + { state: "closed", draft: true, merged: false, expected: "Closed" }, + { state: "closed", draft: false, merged: true, expected: "Merged" }, + ].forEach((status) => { + Object.assign(item, status); + state.renderedViewHtml = ""; + render(); + result.states.push({ + expected: status.expected, + badge: document.querySelector("[data-pr-status-badge]")?.textContent?.trim(), + navMeta: document.querySelector(".pr-diff-nav-item.active .pr-diff-nav-copy > span")?.textContent, + }); + }); + return result; + }, prContextAgentKey); + if (prOriginMetadata.itemTitle !== "Smoke pull request" || + prOriginMetadata.navTitle !== "Smoke pull request" || + prOriginMetadata.detailTitle !== "Smoke pull request" || + prOriginMetadata.states.some((status) => ( + status.badge !== status.expected || !status.navMeta?.includes(status.expected) + ))) { + throw new Error(`PR title or status did not reflect origin metadata: ${JSON.stringify(prOriginMetadata)}`); + } const selectableLines = prContextPage.locator("[data-select-pr-diff-line]"); if (await selectableLines.count() !== 2) throw new Error("PR detail did not expose selectable changed lines"); const firstLine = selectableLines.nth(0); diff --git a/docs/PROJECT_STATUS.md b/docs/PROJECT_STATUS.md index d955987..730ba93 100644 --- a/docs/PROJECT_STATUS.md +++ b/docs/PROJECT_STATUS.md @@ -55,6 +55,7 @@ Key references: | Agent attachments | Structured agent results can include PR/document/image attachments, persisted in `.attachments.json` and mirrored into `memory.jsonl`, surfaced from chat with a `ctrl+a` navigable list | Durable links to artifacts survive later runs and memory rebuilds instead of living only in a single assistant message | | Remote artifact rendering | Render attached HTML in an origin-isolated iframe with a restrictive content policy and package explicitly referenced, allowlisted workspace web assets; render sanitized Markdown Mermaid fences with a pinned, conditional CDN loader in strict mode | Interactive lessons and shared course assets remain usable without granting generated HTML access to Tycho or browser storage, and ordinary Markdown does not pay the Mermaid download cost | | Pull request review | Agent-scoped PR diff inspection remains available; the cross-agent Review Inbox is paused because its eager aggregation is too slow and unresponsive | Redesign inbox discovery and loading around bounded, incremental work before restoring its route; retain GitHub App and `gh` compatibility | +| Agent pull request catalog | Persist canonical PR references and origin title/status metadata in an agent-owned `.pull_request_catalog.json` sidecar; keep ordinary agent PR listing network-free | Opening one agent reads only that agent's catalog; displayed titles and Open/Draft/Closed/Merged state come from cached GitHub metadata, refreshes remain explicit, and archiving moves the catalog with the agent | | Conversation block scrolling | Initial chat load bottom-aligns the latest block when it fits, oversized blocks start at row 1, and navigation scrolls only enough to reveal the selected block | The selected label/cursor must remain visible and predictable while keeping surrounding recent context on first open | | Conversation viewport offsets | Block `line_offset` / `line_height` are derived from the final rendered rows; long unbroken preview tokens are hard-wrapped before entering the viewport, and footer debug is computed after viewport sync | Bubbles `Viewport` counts newline-separated lines, while terminals visually wrap long tokens; stale or mismatched offsets cause misleading `visible 0/0` debug and cropped selected blocks | | Inquiry submission | Gated review step inside a rounded box | Prevents accidental structured submissions | diff --git a/docs/PULL_REQUEST_DIFFS.md b/docs/PULL_REQUEST_DIFFS.md index 623bb63..970377a 100644 --- a/docs/PULL_REQUEST_DIFFS.md +++ b/docs/PULL_REQUEST_DIFFS.md @@ -8,6 +8,8 @@ Tycho prefers a Tycho GitHub App user session obtained through OAuth device flow Review posting remains off by default. Operators must also set `TYCHO_GITHUB_WRITE_ENABLED=true`, save a draft bound to the current base and head, and confirm the mutation. GitHub remains the final permission authority; a token without `Pull requests: write` receives a sanitized failure. +Agent-scoped PR discovery persists canonical references and compact metadata in an agent-owned `~/.tycho/logs/agents/.pull_request_catalog.json` sidecar. Opening an agent's PR list reads only that agent's catalog and does not issue one GitHub request per PR. The cached GitHub title replaces attachment-supplied display text, and the list/detail surfaces show Open, Draft, Closed, or Merged state. Existing saved snapshots seed missing catalog metadata without another patch fetch. **Refresh metadata** updates the catalog explicitly; fetching one or all diffs remains a separate patch operation. Archiving an agent moves its catalog, backup, and lock sidecars with its other logs. + ## Workflow ```mermaid diff --git a/docs/REMOTE_SERVER.md b/docs/REMOTE_SERVER.md index 20759e1..47f4620 100644 --- a/docs/REMOTE_SERVER.md +++ b/docs/REMOTE_SERVER.md @@ -387,7 +387,8 @@ Conversation entries are projected from `AgentChatLog#chat_blocks` when availabl | `DELETE` | `/agents/{key}` | Archive one idle managed agent. | | `POST` | `/agents/archive` | Archive multiple idle managed agents from a `keys` array, returning archived, skipped, and failed keys. | | `GET` | `/agents/{key}/conversation` | Read the rendered conversation blocks for one agent. | -| `GET` | `/agents/{key}/pull-requests` | List GitHub pull request links detected from one agent's attachments with snapshot freshness metadata. | +| `GET` | `/agents/{key}/pull-requests` | List GitHub pull requests from that agent's persistent local catalog, including cached origin title and status, without waiting on GitHub. | +| `POST` | `/agents/{key}/pull-requests/metadata/refresh` | Explicitly refresh GitHub metadata for the agent's cataloged pull requests without fetching patches. | | `GET` | `/agents/{key}/pull-requests/{id}/diff` | Read one saved pull request diff snapshot. | | `POST` | `/agents/{key}/pull-requests/{id}/refresh` | Fetch current PR metadata and patch content, then save a fresh diff snapshot. | | `POST` | `/agents/{key}/pull-requests/refresh` | Refresh every detected pull request diff for one agent. | diff --git a/lib/hq/domain/managed_agent.rb b/lib/hq/domain/managed_agent.rb index 8ecb886..1b43009 100644 --- a/lib/hq/domain/managed_agent.rb +++ b/lib/hq/domain/managed_agent.rb @@ -627,6 +627,10 @@ def attachments_path derived_log_path("attachments.json") end + def pull_request_catalog_path + derived_log_path("pull_request_catalog.json") + end + def invalidate_derived_logs! [conversation_log_path, system_log_path].each do |path| FileUtils.rm_f(path) @@ -640,6 +644,9 @@ def log_files system_log_path, memory_path, attachments_path, + pull_request_catalog_path, + "#{pull_request_catalog_path}.bak", + "#{pull_request_catalog_path}.lock", invalid_structured_output_file_path, status_file_path, last_message_file_path, diff --git a/lib/hq/domain/pull_request_diff.rb b/lib/hq/domain/pull_request_diff.rb index 3c4da5d..0498ba7 100644 --- a/lib/hq/domain/pull_request_diff.rb +++ b/lib/hq/domain/pull_request_diff.rb @@ -105,6 +105,103 @@ def with_lock end end + class Catalog + STORE_VERSION = 1 + METADATA_KEYS = %w[ + title url state draft mergeable mergeable_state merged author base_sha head_sha base_ref head_ref + file_count additions deletions remote_updated_at + ].freeze + + def initialize(path:) + @path = path + end + + def all + parsed = FileStore.read_json(@path, fallback: {}) + return {} unless parsed.is_a?(Hash) && parsed["version"] == STORE_VERSION + + parsed.fetch("entries", {}) + rescue StandardError => e + HQ.logger.warn("PRCatalog") { "Failed to load PR catalog from #{@path}: #{e.class} - #{e.message}" } + {} + end + + def discover(references, metadata_by_id: {}) + update do |entries| + changed = false + Array(references).each do |reference| + unless entries.key?(reference.id) + entries[reference.id] = reference_entry(reference, Time.now.iso8601) + changed = true + end + metadata = metadata_by_id[reference.id] + next unless metadata.is_a?(Hash) && !entries[reference.id]["metadata"].is_a?(Hash) + + cache_metadata( + entries[reference.id], + metadata, + metadata["fetched_at"] || Time.now.iso8601, + source: "snapshot" + ) + changed = true + end + changed + end + end + + def save_metadata(reference, metadata) + save_all_metadata([[reference, metadata]]) + end + + def save_all_metadata(items) + items = Array(items) + refreshed_at = Time.now.iso8601 + update do |entries| + items.each do |reference, metadata| + entries[reference.id] ||= reference_entry(reference, refreshed_at) + cache_metadata(entries[reference.id], metadata, refreshed_at, source: "github") + end + items.any? + end + end + + private + + def reference_entry(reference, discovered_at) + { + "id" => reference.id, + "provider" => reference.provider, + "repository" => reference.repository, + "number" => reference.number, + "url" => reference.url, + "discovered_at" => discovered_at + }.compact + end + + def cache_metadata(entry, metadata, refreshed_at, source:) + entry["metadata"] = metadata.to_h + .select { |key, _value| METADATA_KEYS.include?(key.to_s) } + .transform_keys(&:to_s) + entry["metadata_refreshed_at"] = refreshed_at + entry["metadata_source"] = source + end + + def update + FileUtils.mkdir_p(File.dirname(@path)) + File.open("#{@path}.lock", File::RDWR | File::CREAT, 0o600) do |lock| + lock.flock(File::LOCK_EX) + entries = all + changed = yield entries + if changed + FileStore.write_json(@path, { "version" => STORE_VERSION, "entries" => entries }) + end + entries + ensure + lock.flock(File::LOCK_UN) rescue nil + end + end + end + class GitHubProvider def initialize(client: GitHubAPIClient.new) @client = client @@ -243,6 +340,7 @@ def snapshot_for(reference, provider: GitHubProvider.new, metadata: nil) "description" => metadata["body"] || reference.description, "state" => metadata["state"], "draft" => metadata["draft"], + "merged" => metadata["merged"], "author" => metadata["author"], "base_sha" => metadata["base_sha"], "head_sha" => metadata["head_sha"], @@ -262,9 +360,14 @@ def snapshot_for(reference, provider: GitHubProvider.new, metadata: nil) }.compact end - def reference_payload(reference, snapshot: nil, metadata: nil, error: nil) + def reference_payload(reference, snapshot: nil, metadata: nil, freshness_metadata: metadata, error: nil) payload = reference.to_h - payload["snapshot"] = snapshot_summary(snapshot, metadata:) + if metadata.is_a?(Hash) + Catalog::METADATA_KEYS.each do |key| + payload[key] = metadata[key] if metadata.key?(key) + end + end + payload["snapshot"] = snapshot_summary(snapshot, metadata: freshness_metadata) payload["error"] = error if error payload end diff --git a/lib/hq/remote_server.rb b/lib/hq/remote_server.rb index 5223c2a..e831c13 100644 --- a/lib/hq/remote_server.rb +++ b/lib/hq/remote_server.rb @@ -386,6 +386,9 @@ def route(service, method, path, body, request = nil) return ok(memory_rebuild: service.rebuild_agent_memory(key)) end return ok(pull_requests: service.agent_pull_requests(key)) if method == "GET" && tail == ["pull-requests"] + if method == "POST" && tail == ["pull-requests", "metadata", "refresh"] + return ok(service.refresh_agent_pull_request_metadata(key)) + end return ok(service.refresh_agent_pull_requests(key)) if method == "POST" && tail == ["pull-requests", "refresh"] if tail.length == 3 && tail.first == "pull-requests" && tail[2] == "diff" return ok(diff: service.agent_pull_request_diff(key, tail[1])) if method == "GET" @@ -1694,17 +1697,40 @@ def agent_pull_requests(key) ensure_github_enabled! agent = find_agent!(key) references = PullRequestDiff.references_for_agent(agent) - store = @pull_request_diff_store - provider = PullRequestDiff::GitHubProvider.new(client: @github_client) + snapshots = @pull_request_diff_store.all + catalog = pull_request_catalog(agent).discover(references, metadata_by_id: snapshots) references.map do |reference| - snapshot = store.fetch(reference.id) - begin - metadata = provider.metadata(reference) - PullRequestDiff.reference_payload(reference, snapshot:, metadata:) - rescue PullRequestDiff::Error => e - PullRequestDiff.reference_payload(reference, snapshot:, error: e.message) - end + pull_request_reference_payload(reference, catalog[reference.id], snapshots[reference.id]) + end + end + + def refresh_agent_pull_request_metadata(key) + ensure_github_enabled! + agent = find_agent!(key) + references = PullRequestDiff.references_for_agent(agent) + catalog_store = pull_request_catalog(agent) + catalog_store.discover(references) + refreshed = [] + failed = [] + references.each do |reference| + refreshed << [reference, github_provider.metadata(reference)] + rescue PullRequestDiff::Error => e + failed << { + id: reference.id, + repository: reference.repository, + number: reference.number, + error: e.message + } end + catalog = catalog_store.save_all_metadata(refreshed) + snapshots = @pull_request_diff_store.all + { + pull_requests: references.map do |reference| + pull_request_reference_payload(reference, catalog[reference.id], snapshots[reference.id]) + end, + refreshed: refreshed.map { |reference, _metadata| reference.id }, + failed: + } end def agent_pull_request_diff(key, id) @@ -3224,15 +3250,39 @@ def github_provider PullRequestDiff::GitHubProvider.new(client: @github_client) end + def pull_request_reference_payload(reference, entry, snapshot) + entry ||= {} + metadata = entry["metadata"] + freshness_metadata = metadata if entry["metadata_source"] == "github" + payload = PullRequestDiff.reference_payload(reference, snapshot:, metadata:, freshness_metadata:) + if metadata.is_a?(Hash) + payload["metadata_refreshed_at"] = entry["metadata_refreshed_at"] + end + payload + end + + def pull_request_catalog(agent) + PullRequestDiff::Catalog.new(path: agent.pull_request_catalog_path) + end + + def persist_pull_request_metadata(reference, metadata) + return if reference.agent_key.to_s.empty? + + agent = load_agents.find { |candidate| candidate.key == reference.agent_key } + pull_request_catalog(agent).save_metadata(reference, metadata) if agent + end + def refresh_pull_request_snapshot(reference) refresh_pull_request_snapshot_fetch(reference).fetch(:snapshot) end def refresh_pull_request_snapshot_fetch(reference) - coalesce_pull_request_fetch(reference, "snapshot") do + refreshed = coalesce_pull_request_fetch(reference, "snapshot") do metadata, pull = github_provider.metadata_with_pull(reference) - { snapshot: save_pull_request_snapshot(reference, metadata), pull: } + { snapshot: save_pull_request_snapshot(reference, metadata), pull:, metadata: } end + persist_pull_request_metadata(reference, refreshed.fetch(:metadata)) + refreshed end def refresh_pull_request_fetch(reference) diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index a939896..f8e5784 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -5375,8 +5375,10 @@ function renderPullRequestDiffNavItem(agent, item, selectedId) { const active = item.id === selectedId; const snapshot = item.snapshot || null; const fetching = isPullRequestDiffFetching(agent.key, item.id); + const prStatus = pullRequestStatus(item); const meta = [ item.repository ? `${item.repository}#${item.number}` : `PR #${item.number}`, + prStatus?.label, pullRequestFreshnessLabel(item), ].filter(Boolean).join(" / "); return ` @@ -5397,6 +5399,7 @@ function renderPullRequestDiffDetail(agent, item, diff, options = {}) { const snapshot = item.snapshot || null; const stale = snapshot?.fresh === false; const loading = isPullRequestDiffFetching(agent.key, item.id); + const prStatus = pullRequestStatus(item); const expandAll = state.prDiffExpandAll[agent.key] !== false; const expandLabel = expandAll ? "Collapse all" : "Open all"; const expandIcon = expandAll ? "listChevronsDownUp" : "listChevronsUpDown"; @@ -5411,6 +5414,7 @@ function renderPullRequestDiffDetail(agent, item, diff, options = {}) { ${escapeHtml(item.title || `${item.repository}#${item.number}`)} ${escapeHtml([item.repository, item.number ? `#${item.number}` : "", pullRequestFreshnessLabel(item)].filter(Boolean).join(" / "))}
+ ${prStatus ? `${statusBadge(prStatus.label, prStatus.className, "chip")}` : ""} ${snapshot ? statusBadge(stale ? "Stale" : snapshot.fresh === true ? "Fresh" : "Snapshot", stale ? "need" : "done", "chip") : statusBadge("Not fetched", "need", "chip")} ${snapshot?.head_sha ? metadataBadge(shortSha(snapshot.head_sha), "chip") : ""} ${snapshot?.truncated ? statusBadge("Truncated", "need", "chip") : ""} @@ -7371,6 +7375,14 @@ function pullRequestFreshnessLabel(item) { return snapshot.fetched_at ? `fetched ${timeShort(snapshot.fetched_at)}` : "snapshot"; } +function pullRequestStatus(item) { + if (item?.merged === true) return { key: "merged", label: "Merged", className: "info" }; + if (item?.state === "closed") return { key: "closed", label: "Closed", className: "detail" }; + if (item?.draft === true) return { key: "draft", label: "Draft", className: "need" }; + if (item?.state === "open") return { key: "open", label: "Open", className: "done" }; + return null; +} + function shortSha(value) { const text = String(value || ""); return text.length > 8 ? text.slice(0, 8) : text; @@ -13902,10 +13914,14 @@ async function refreshAgentPullRequests(agentKey) { if (!agentKey) return; try { - setConnection("Refreshing pull requests"); - await ensureAgentPullRequests(agentKey, true); + setConnection("Refreshing PR metadata"); + const data = await apiPost(`/agents/${encodeURIComponent(agentKey)}/pull-requests/metadata/refresh`); + state.pullRequests[agentKey] = { loading: false, items: data.pull_requests || [] }; state.renderedViewHtml = ""; - setConnection("Pull requests refreshed"); + const failures = Array.isArray(data.failed) ? data.failed.length : 0; + setConnection(failures + ? `PR metadata refreshed with ${failures} failure${failures === 1 ? "" : "s"}` + : "PR metadata refreshed"); render(); } catch (error) { setConnection(error.message); diff --git a/test/remote_server_test.rb b/test/remote_server_test.rb index 15bf023..f6b562f 100644 --- a/test/remote_server_test.rb +++ b/test/remote_server_test.rb @@ -26,6 +26,7 @@ def run! assert_remote_inquiry_payload_has_stable_id_and_guarded_answer assert_remote_agent_payload_includes_attachments assert_remote_agent_pull_request_diff_payload + assert_agent_pull_request_listing_avoids_eager_metadata_requests assert_concurrent_pull_request_diff_refreshes_are_coalesced assert_pull_request_review_refresh_reuses_metadata assert_pull_request_feature_gate_and_global_inbox @@ -933,8 +934,10 @@ def assert_remote_agent_pull_request_diff_payload reference = payload.first assert(reference["repository"] == "example/web", "expected GitHub repository to be parsed") assert(reference["number"] == 123, "expected GitHub PR number to be parsed") - assert(reference["error"].to_s.length.positive?, - "expected metadata errors to be reported without hiding PR references") + assert(reference["error"].nil?, "expected ordinary PR listing to avoid unavailable GitHub metadata") + metadata_refresh = service.refresh_agent_pull_request_metadata(created[:key]) + assert(metadata_refresh[:failed].length == 1, + "expected explicit metadata refresh errors to be reported without hiding PR references") snapshot = { "id" => reference["id"], @@ -943,7 +946,9 @@ def assert_remote_agent_pull_request_diff_payload "repository" => "example/web", "number" => 123, "url" => "https://github.com/example/web/pull/123", - "title" => "Example PR", + "title" => "Origin PR title", + "state" => "open", + "draft" => true, "head_sha" => "abc1234", "base_sha" => "def5678", "fetched_at" => Time.now.iso8601, @@ -965,6 +970,13 @@ def assert_remote_agent_pull_request_diff_payload } HQ::PullRequestDiff::Store.new.save(snapshot) + listed_from_snapshot = service.agent_pull_requests(created[:key]).first + assert(listed_from_snapshot["title"] == "Origin PR title" && + listed_from_snapshot["state"] == "open" && listed_from_snapshot["draft"] == true, + "expected saved origin metadata to backfill the agent PR catalog and listing") + assert(!listed_from_snapshot.fetch("snapshot").key?("fresh"), + "expected snapshot-seeded metadata to leave remote freshness unknown") + diff = service.agent_pull_request_diff(created[:key], reference["id"]) assert(diff["files"].first["path"] == "lib/example.rb", "expected saved PR diff snapshot to be returned") @@ -974,26 +986,157 @@ def assert_remote_agent_pull_request_diff_payload end end + def assert_agent_pull_request_listing_avoids_eager_metadata_requests + with_remote_temp_store do |dir| + workspace = File.join(dir, "workspace") + write_project_workspace(workspace) + registry = registry_for_project(dir, workspace) + client = FakeGitHubReviewClient.new + snapshot_store = CountingPullRequestDiffStore.new(File.join(dir, "diffs.json")) + service = HQ::RemoteService.new(registry:, github_client: client, pull_request_diff_store: snapshot_store) + created = service.create_agent( + "project_key" => "web", + "template_key" => "custom", + "name" => "Many pull requests", + "prompt" => "Review pull requests.", + "agent" => "codex" + ) + agent = HQ::AgentStore.new(registry.projects).load.find { |item| item.key == created[:key] } + memory = HQ::AgentMemory.new(agent) + 3.times do |index| + memory.append_attachment!( + { + "kind" => "link", + "title" => "PR #{index + 1}", + "url" => "https://github.com/example/web/pull/#{index + 1}" + }, + created_at: Time.parse("2026-08-09 13:00:0#{index}") + ) + end + + listed = service.agent_pull_requests(created[:key]) + metadata_requests = client.requests.count { |kind, path| kind == :get_json && path.include?("/pulls/") } + + assert(listed.length == 3, "expected every attached pull request to be listed") + assert(metadata_requests.zero?, + "expected ordinary PR listing to avoid one blocking GitHub metadata request per pull request") + assert(snapshot_store.all_calls == 1, + "expected ordinary PR listing to parse the shared diff snapshot store only once") + + server = HQ::RemoteServer.new + response = server.send( + :route, + service, + "POST", + "/agents/#{created[:key]}/pull-requests/metadata/refresh", + {}, + nil + ) + refreshed = response[:body] + metadata_requests = client.requests.count { |kind, path| kind == :get_json && path.include?("/pulls/") } + assert(metadata_requests == 3, "expected explicit metadata refresh to request each pull request once") + assert(refreshed[:pull_requests].all? { |item| item["title"] == "Shared PR" }, + "expected refreshed GitHub metadata to be returned from the persistent catalog") + assert(refreshed[:pull_requests].all? { |item| item["metadata_refreshed_at"].to_s.length.positive? }, + "expected cached PR metadata to expose its refresh time") + catalog = JSON.parse(File.read(agent.pull_request_catalog_path)) + assert(catalog.fetch("entries").length == 3, + "expected newly discovered pull requests to be persisted in the agent-owned PR catalog") + assert(!File.exist?(File.join(HQ::AGENT_LOGS_DIR, "pull_request_catalog.json")), + "expected agent PR listing to avoid a shared cross-agent catalog") + + second_created = service.create_agent( + "project_key" => "web", + "template_key" => "custom", + "name" => "Other pull requests", + "prompt" => "Review another list.", + "agent" => "codex" + ) + second_agent = HQ::AgentStore.new(registry.projects).load.find { |item| item.key == second_created[:key] } + HQ::AgentMemory.new(second_agent).append_attachment!( + { + "kind" => "link", + "title" => "Other PR", + "url" => "https://github.com/other/repository/pull/99" + }, + created_at: Time.parse("2026-08-09 13:01:00") + ) + requests_before_second_list = client.requests.length + second_list = service.agent_pull_requests(second_created[:key]) + second_catalog = JSON.parse(File.read(second_agent.pull_request_catalog_path)) + assert(second_list.length == 1 && client.requests.length == requests_before_second_list, + "expected another agent's PR list to remain network-free") + assert(second_agent.pull_request_catalog_path != agent.pull_request_catalog_path, + "expected each agent to own a distinct PR catalog path") + assert(second_catalog.fetch("entries").length == 1 && catalog.fetch("entries").length == 3, + "expected one agent's PR discovery to stay isolated from every other catalog") + + restarted = HQ::RemoteService.new(registry:, github_client: FakeUnavailableGitHubClient.new) + cached = restarted.agent_pull_requests(created[:key]) + assert(cached.length == 3 && cached.all? { |item| item["title"] == "Shared PR" }, + "expected PR references and metadata to survive a Remote server restart") + + catalog_paths = [ + agent.pull_request_catalog_path, + "#{agent.pull_request_catalog_path}.bak", + "#{agent.pull_request_catalog_path}.lock" + ] + archive = agent.archive_logs!(File.join(dir, "archive")) + assert(catalog_paths.all? { |path| File.exist?(File.join(archive, File.basename(path))) }, + "expected agent archive to move the PR catalog, backup, and lock sidecars") + assert(catalog_paths.none? { |path| File.exist?(path) }, + "expected agent archive to leave no active PR catalog sidecars behind") + end + end + def assert_concurrent_pull_request_diff_refreshes_are_coalesced with_remote_temp_store do |dir| workspace = File.join(dir, "workspace") write_project_workspace(workspace) client = BlockingGitHubDiffClient.new + registry = registry_for_project(dir, workspace) service = HQ::RemoteService.new( - registry: registry_for_project(dir, workspace), + registry:, github_client: client, pull_request_diff_store: HQ::PullRequestDiff::Store.new(File.join(dir, "diffs.json")) ) - reference = HQ::PullRequestDiff.reference_from_url("https://github.com/example/web/pull/123") - first = Thread.new { service.send(:refresh_pull_request_snapshot, reference) } + agents = 2.times.map do |index| + created = service.create_agent( + "project_key" => "web", + "template_key" => "custom", + "name" => "Shared PR agent #{index + 1}", + "prompt" => "Review the shared PR.", + "agent" => "codex" + ) + agent = HQ::AgentStore.new(registry.projects).load.find { |item| item.key == created[:key] } + HQ::AgentMemory.new(agent).append_attachment!( + { + "kind" => "link", + "title" => "Shared PR", + "url" => "https://github.com/example/web/pull/123" + }, + created_at: Time.parse("2026-08-09 13:02:0#{index}") + ) + agent + end + references = agents.map { |agent| HQ::PullRequestDiff.references_for_agent(agent).fetch(0) } + first = Thread.new { service.send(:refresh_pull_request_snapshot, references.fetch(0)) } client.metadata_started.pop - second = Thread.new { service.send(:refresh_pull_request_snapshot, reference) } + second = Thread.new { service.send(:refresh_pull_request_snapshot, references.fetch(1)) } + + deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + 2 + Thread.pass until second.status == "sleep" || Process.clock_gettime(Process::CLOCK_MONOTONIC) >= deadline + follower_waiting = second.status == "sleep" client.release! [first, second].each(&:value) + assert(follower_waiting, "expected the second agent refresh to join the in-flight PR fetch") assert(client.metadata_requests == 1 && client.diff_requests == 1, "expected concurrent identical diff refreshes to share one metadata and diff fetch") + catalogs = agents.map { |agent| JSON.parse(File.read(agent.pull_request_catalog_path)) } + assert(catalogs.all? { |catalog| catalog.dig("entries", references.first.id, "metadata", "title") == "Shared PR" }, + "expected every coalesced caller to persist metadata in its own agent catalog") end end @@ -6262,6 +6405,20 @@ def release! end end + class CountingPullRequestDiffStore < HQ::PullRequestDiff::Store + attr_reader :all_calls + + def initialize(path) + super + @all_calls = 0 + end + + def all + @all_calls += 1 + super + end + end + class FakeUnavailableGitHubClient def enabled? true From f040cce84eb9c32ff16f608207def4fcf4ae0776 Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Sun, 9 Aug 2026 22:34:38 +0700 Subject: [PATCH 07/12] Load saved PR diffs without conversation delay --- bin/remote-ui-smoke | 65 ++++++++++++++++++++++++++++++++++ lib/hq/remote_ui/assets/app.js | 17 +++++---- 2 files changed, 76 insertions(+), 6 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index 0c2c86b..af62a49 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -1748,6 +1748,71 @@ def write_smoke_script(path) ))) { throw new Error(`PR title or status did not reflect origin metadata: ${JSON.stringify(prOriginMetadata)}`); } + const delayedConversationPattern = `**/agents/${encodeURIComponent(prContextAgentKey)}/conversation`; + const originalPullRequestHash = await prContextPage.evaluate(() => location.hash); + let releaseDelayedConversation; + const delayedConversationGate = new Promise((resolve) => { + releaseDelayedConversation = resolve; + }); + const delayedConversationHandler = async (route) => { + await delayedConversationGate; + await route.continue(); + }; + await prContextPage.route(delayedConversationPattern, delayedConversationHandler); + const secondPullRequestId = await prContextPage.evaluate((key) => { + const current = state.pullRequests[key].items[0]; + const id = `${current.id}-navigation-latency`; + state.pullRequests[key].items.push({ + ...current, + id, + number: Number(current.number || 0) + 1, + title: "Navigation latency fixture", + }); + state.renderedViewHtml = ""; + render(); + return id; + }, prContextAgentKey); + const navigationDiffPattern = `**/pull-requests/${encodeURIComponent(secondPullRequestId)}/diff`; + let resolveNavigationDiffRequest; + const navigationDiffRequest = new Promise((resolve) => { + resolveNavigationDiffRequest = resolve; + }); + const navigationDiffHandler = async (route) => { + resolveNavigationDiffRequest(); + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ diff: { id: secondPullRequestId, snapshot_id: "navigation-latency", files: [] } }), + }); + }; + await prContextPage.route(navigationDiffPattern, navigationDiffHandler); + const delayedConversationResponse = prContextPage.waitForResponse( + (response) => response.url().endsWith(`/agents/${encodeURIComponent(prContextAgentKey)}/conversation`), + { timeout: 10_000 } + ); + await prContextPage.click(`.pr-diff-nav-item[href*='${encodeURIComponent(secondPullRequestId)}']`); + await prContextPage.waitForFunction( + (id) => location.hash.includes(encodeURIComponent(id)), + secondPullRequestId, + { timeout: 10_000 } + ); + let navigationDiffTimeout; + await Promise.race([ + navigationDiffRequest, + new Promise((_resolve, reject) => { + navigationDiffTimeout = setTimeout( + () => reject(new Error("PR diff request waited behind unrelated route data")), + 10_000 + ); + }), + ]); + clearTimeout(navigationDiffTimeout); + releaseDelayedConversation(); + await delayedConversationResponse; + await prContextPage.unroute(delayedConversationPattern, delayedConversationHandler); + await prContextPage.unroute(navigationDiffPattern, navigationDiffHandler); + await prContextPage.goto(`${baseUrl}/ui${originalPullRequestHash}`, { waitUntil: "networkidle" }); + await prContextPage.waitForSelector("[data-select-pr-diff-line]", { state: "visible", timeout: 10_000 }); const selectableLines = prContextPage.locator("[data-select-pr-diff-line]"); if (await selectableLines.count() !== 2) throw new Error("PR detail did not expose selectable changed lines"); const firstLine = selectableLines.nth(0); diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index f8e5784..11314ad 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -1758,6 +1758,7 @@ async function refresh(options = {}) { async function ensureRouteData(options = {}) { const route = parseRoute(); + let agentShellData = Promise.resolve(); if (route.type === "tab" && route.tab === "settings") { await Promise.all([ ensureResponseStyle(options.forceResponseStyle || options.force), @@ -1768,23 +1769,27 @@ async function ensureRouteData(options = {}) { await ensureAgentDetail(route.key, options.forceAgent || options.force); const agent = findAgent(route.key); if (agent) { - await Promise.all([ + agentShellData = Promise.all([ ensureConversation(agent, options.forceConversation), ensureSkillsForProject(agent.project_key, agent.agent, { force: options.force }), ensureProject(agent.project_key), ]); } } + if (route.type !== "agentPullRequests") await agentShellData; if (route.type === "agentAttachment") { await ensureAttachment(route.attachmentId, options.forceAttachment); await ensureAttachmentPreview(route.attachmentId, options.forceAttachment); } if (route.type === "agentPullRequests") { - await ensureAgentPullRequests(route.key, options.forcePullRequests); - const refs = state.pullRequests[route.key]?.items || []; - const selected = selectedPullRequestId(route.key, route.pullRequestId); - const hasSnapshot = refs.find((item) => item.id === selected)?.snapshot; - if (selected && hasSnapshot) await ensurePullRequestDiff(route.key, selected, options.forcePullRequestDiff); + const pullRequestData = (async () => { + await ensureAgentPullRequests(route.key, options.forcePullRequests); + const refs = state.pullRequests[route.key]?.items || []; + const selected = selectedPullRequestId(route.key, route.pullRequestId); + const hasSnapshot = refs.find((item) => item.id === selected)?.snapshot; + if (selected && hasSnapshot) await ensurePullRequestDiff(route.key, selected, options.forcePullRequestDiff); + })(); + await Promise.all([agentShellData, pullRequestData]); } if (route.type === "agentForm" && route.mode === "create") { await ensureProject(route.projectKey, true); From ff6e41d2db195dea37ec59d93a4b7e796b304dbd Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Sun, 9 Aug 2026 23:08:47 +0700 Subject: [PATCH 08/12] Cache saved PR diffs for fast switching --- bin/remote-ui-smoke | 337 ++++++++++++++++++++++++++++- docs/PROJECT_STATUS.md | 1 + docs/PULL_REQUEST_DIFFS.md | 2 + lib/hq/domain/pull_request_diff.rb | 49 ++++- lib/hq/remote_ui/assets/app.css | 6 + lib/hq/remote_ui/assets/app.js | 258 ++++++++++++++++++++-- test/pull_request_diff_test.rb | 21 ++ 7 files changed, 638 insertions(+), 36 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index af62a49..4be4553 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -1813,6 +1813,331 @@ def write_smoke_script(path) await prContextPage.unroute(navigationDiffPattern, navigationDiffHandler); await prContextPage.goto(`${baseUrl}/ui${originalPullRequestHash}`, { waitUntil: "networkidle" }); await prContextPage.waitForSelector("[data-select-pr-diff-line]", { state: "visible", timeout: 10_000 }); + const preloadPullRequestId = await prContextPage.evaluate((key) => { + const current = state.pullRequests[key].items[0]; + const id = `${current.id}-background-preload`; + state.pullRequests[key].items.push({ + ...current, + id, + number: Number(current.number || 0) + 3, + title: "Background preload fixture", + }); + return id; + }, prContextAgentKey); + const preloadPattern = `**/pull-requests/${encodeURIComponent(preloadPullRequestId)}/diff`; + let resolvePreloadRequest; + const preloadRequest = new Promise((resolve) => { resolvePreloadRequest = resolve; }); + const preloadHandler = async (route) => { + resolvePreloadRequest(); + await route.fulfill({ + status: 200, + contentType: "application/json", + body: JSON.stringify({ diff: { id: preloadPullRequestId, snapshot_id: "background-preload", files: [] } }), + }); + }; + await prContextPage.route(preloadPattern, preloadHandler); + const preloadHash = await prContextPage.evaluate((key) => { + const route = parseRoute(); + preloadSavedPullRequestDiffs(key, state.pullRequests[key].items, selectedPullRequestId(key, route.pullRequestId)); + return location.hash; + }, prContextAgentKey); + let preloadTimeout; + await Promise.race([ + preloadRequest, + new Promise((_resolve, reject) => { + preloadTimeout = setTimeout( + () => reject(new Error("Saved PR diff did not preload in the background")), + 10_000 + ); + }), + ]); + clearTimeout(preloadTimeout); + await prContextPage.waitForFunction( + ({ key, id }) => { + const diff = state.pullRequestDiffs[pullRequestDiffKey(key, id)]; + return diff && !diff.loading && !diff.error; + }, + { key: prContextAgentKey, id: preloadPullRequestId }, + { timeout: 10_000 } + ); + const preloadStayedOnSelectedPullRequest = await prContextPage.evaluate((hash) => location.hash === hash, preloadHash); + await prContextPage.unroute(preloadPattern, preloadHandler); + if (!preloadStayedOnSelectedPullRequest) throw new Error("Background PR preload changed the selected route"); + const prioritizedFetchContract = await prContextPage.evaluate(async (key) => { + const originalApiGet = apiGet; + const originalApiPost = apiPost; + const originalEnsureAgentPullRequests = ensureAgentPullRequests; + const ids = ["queue-background-1", "queue-background-2", "queue-foreground"]; + const starts = []; + const releases = {}; + const waitUntil = async (predicate, message) => { + const deadline = Date.now() + 10_000; + while (!predicate()) { + if (Date.now() > deadline) throw new Error(message); + await new Promise((resolve) => setTimeout(resolve, 10)); + } + }; + apiGet = async (path) => { + const id = ids.find((candidate) => path.includes(candidate)); + if (!id) return originalApiGet(path); + starts.push(id); + if (id === ids[2]) return { diff: { id, snapshot_id: id, files: [] } }; + await new Promise((resolve) => { releases[id] = resolve; }); + return { diff: { id, snapshot_id: id, files: [] } }; + }; + let refreshGetRelease; + let refreshPostCount = 0; + apiPost = async (path, body) => { + if (!path.includes("queue-refresh")) return originalApiPost(path, body); + refreshPostCount += 1; + return { diff: { id: "queue-refresh", snapshot_id: "queue-refresh-post", files: [] } }; + }; + try { + const backgroundPromises = ids.map((id) => enqueuePullRequestDiffFetch(key, id, false, false, { background: true })); + await waitUntil(() => starts.length === 2, "background preloads did not fill their bounded capacity"); + const selectedPromise = enqueuePullRequestDiffFetch(key, ids[2]); + await waitUntil(() => starts.includes(ids[2]), "selected PR remained queued behind background preloads"); + const foregroundStartedBeforeRelease = !releases[ids[0]] && !releases[ids[1]] + ? false + : starts[2] === ids[2]; + releases[ids[0]](); + releases[ids[1]](); + await Promise.all([...backgroundPromises, selectedPromise]); + + const staleIds = ["stale-agent-1", "stale-agent-2", "stale-agent-queued"]; + const staleStarts = []; + const staleReleases = {}; + apiGet = async (path) => { + const id = staleIds.find((candidate) => path.includes(candidate)); + if (!id) return originalApiGet(path); + staleStarts.push(id); + await new Promise((resolve) => { staleReleases[id] = resolve; }); + return { diff: { id, snapshot_id: id, files: [] } }; + }; + const stalePromises = staleIds.map((id) => ( + enqueuePullRequestDiffFetch(`${key}-stale`, id, false, false, { background: true }) + )); + await waitUntil(() => staleStarts.length === 2, "stale agent preloads did not fill background capacity"); + await ensureRouteData(); + const staleQueuedRemoved = !state.pullRequestDiffFetches[pullRequestDiffKey(`${key}-stale`, staleIds[2])] && + !staleStarts.includes(staleIds[2]); + staleReleases[staleIds[0]](); + staleReleases[staleIds[1]](); + await Promise.all(stalePromises); + const staleResultsDiscarded = staleIds.every((id) => !state.pullRequestDiffs[pullRequestDiffKey(`${key}-stale`, id)]); + + apiGet = async (path) => { + if (!path.includes("queue-refresh")) return originalApiGet(path); + starts.push("queue-refresh-get"); + await new Promise((resolve) => { refreshGetRelease = resolve; }); + return { diff: { id: "queue-refresh", snapshot_id: "queue-refresh-get", files: [] } }; + }; + const staleGet = enqueuePullRequestDiffFetch(key, "queue-refresh", false, false, { background: true }); + await waitUntil(() => Boolean(refreshGetRelease), "background refresh fixture did not start"); + const explicitRefresh = enqueuePullRequestDiffFetch(key, "queue-refresh", true, true); + refreshGetRelease(); + await Promise.all([staleGet, explicitRefresh]); + const refreshedSnapshot = state.pullRequestDiffs[pullRequestDiffKey(key, "queue-refresh")]?.snapshot_id; + + const currentId = selectedPullRequestId(key, parseRoute().pullRequestId); + const currentKey = pullRequestDiffKey(key, currentId); + const currentDiff = state.pullRequestDiffs[currentKey]; + ensureAgentPullRequests = async () => {}; + apiPost = async (path, body) => { + if (!path.endsWith(`/agents/${encodeURIComponent(key)}/pull-requests/refresh`)) { + return originalApiPost(path, body); + } + return { + refreshed: [ + { ...currentDiff, id: currentId, snapshot_id: "refresh-all-selected" }, + ...Array.from({ length: MAX_CACHED_PR_DIFF_DATA + 2 }, (_, index) => ({ + id: `refresh-all-${index}`, + snapshot_id: `refresh-all-${index}`, + files: [], + })), + ], + failed: [], + }; + }; + await refreshAllPullRequestDiffs(key); + const refreshAllBounded = state.pullRequestDiffDataOrder.size <= MAX_CACHED_PR_DIFF_DATA; + const refreshAllSelectedRetained = state.pullRequestDiffs[currentKey]?.snapshot_id === "refresh-all-selected"; + state.pullRequestDiffs[currentKey] = currentDiff; + rememberPullRequestDiffData(currentKey); + + for (let index = 0; index < MAX_CACHED_PR_DIFF_DATA + 2; index += 1) { + const cacheKey = pullRequestDiffKey(key, `bounded-${index}`); + state.pullRequestDiffs[cacheKey] = { id: `bounded-${index}`, files: [] }; + rememberPullRequestDiffData(cacheKey); + rememberPullRequestDiffPreloadFailure(cacheKey); + } + state.pullRequestDiffs[currentKey] = currentDiff; + rememberPullRequestDiffData(currentKey); + const previousViewers = state.pullRequestDiffViewers; + state.pullRequestDiffViewers = new Map(); + for (let index = 0; index < MAX_CACHED_PR_DIFF_VIEWERS + 2; index += 1) { + const viewer = document.createElement("section"); + viewer.dataset.stateKey = `pr-diff-viewer:${key}:viewer-${index}`; + viewer.dataset.pollContentVersion = "snapshot:expanded"; + cachePullRequestDiffViewer(viewer); + } + const viewerCacheKeys = Array.from(state.pullRequestDiffViewers.keys()); + state.pullRequestDiffViewers = previousViewers; + return { + foregroundStartedBeforeRelease, + staleQueuedRemoved, + staleResultsDiscarded, + refreshPostCount, + refreshedSnapshot, + refreshAllBounded, + refreshAllSelectedRetained, + dataCacheSize: state.pullRequestDiffDataOrder.size, + failureCacheSize: state.pullRequestDiffPreloadFailures.size, + oldestDataEvicted: !state.pullRequestDiffDataOrder.has(pullRequestDiffKey(key, "bounded-0")), + oldestFailureEvicted: !state.pullRequestDiffPreloadFailures.has(pullRequestDiffKey(key, "bounded-0")), + viewerCacheSize: viewerCacheKeys.length, + oldestViewerEvicted: !viewerCacheKeys.some((cacheKey) => cacheKey.includes("viewer-0")), + }; + } finally { + apiGet = originalApiGet; + apiPost = originalApiPost; + ensureAgentPullRequests = originalEnsureAgentPullRequests; + } + }, prContextAgentKey); + if (!prioritizedFetchContract.foregroundStartedBeforeRelease || + !prioritizedFetchContract.staleQueuedRemoved || + !prioritizedFetchContract.staleResultsDiscarded || + prioritizedFetchContract.refreshPostCount !== 1 || + prioritizedFetchContract.refreshedSnapshot !== "queue-refresh-post" || + !prioritizedFetchContract.refreshAllBounded || + !prioritizedFetchContract.refreshAllSelectedRetained || + prioritizedFetchContract.dataCacheSize > 12 || + prioritizedFetchContract.failureCacheSize > 12 || + !prioritizedFetchContract.oldestDataEvicted || + !prioritizedFetchContract.oldestFailureEvicted || + prioritizedFetchContract.viewerCacheSize !== 6 || + !prioritizedFetchContract.oldestViewerEvicted) { + throw new Error(`PR diff queue priority, refresh, or cache bounds failed: ${JSON.stringify(prioritizedFetchContract)}`); + } + const cachedSwitchIds = await prContextPage.evaluate(async (key) => { + const route = parseRoute(); + const firstId = selectedPullRequestId(key, route.pullRequestId); + const firstItem = state.pullRequests[key].items.find((item) => item.id === firstId); + const firstDiff = state.pullRequestDiffs[pullRequestDiffKey(key, firstId)]; + const secondId = `${firstId}-cached-switch`; + state.pullRequests[key].items.push({ + ...firstItem, + id: secondId, + number: Number(firstItem.number || 0) + 2, + title: "Cached switch fixture", + }); + state.pullRequestDiffs[pullRequestDiffKey(key, secondId)] = { + ...firstDiff, + id: secondId, + snapshot_id: "cached-switch", + }; + state.renderedViewHtml = ""; + render(); + document.querySelector("[data-select-pr-diff-line]").click(); + await new Promise((resolve) => requestAnimationFrame(resolve)); + const firstComment = document.querySelector("[data-pr-context-comment]"); + firstComment.value = "Persistent first PR comment"; + firstComment.dispatchEvent(new Event("input", { bubbles: true })); + const firstViewer = document.querySelector("[data-pr-diff-viewer]"); + const scrollStyle = document.createElement("style"); + scrollStyle.id = "cached-pr-scroll-smoke-style"; + scrollStyle.textContent = ".pr-diff-detail { height: 120px !important; overflow-y: auto !important; }"; + document.head.append(scrollStyle); + const spacer = document.createElement("div"); + spacer.style.height = "2000px"; + spacer.dataset.cachedSwitchSpacer = "first"; + firstViewer.append(spacer); + const firstDetail = firstViewer.closest(".pr-diff-detail"); + void firstDetail.scrollHeight; + firstDetail.scrollTop = 140; + firstViewer.dataset.cachedSwitchProbe = "first"; + window.__tychoCachedFirstViewer = firstViewer; + return { firstId, secondId }; + }, prContextAgentKey); + await prContextPage.click(`.pr-diff-nav-item[href*='${encodeURIComponent(cachedSwitchIds.secondId)}']`); + await prContextPage.waitForSelector(`[data-pr-diff-viewer][data-pull-request-id='${cachedSwitchIds.secondId}']`, { state: "visible", timeout: 10_000 }); + await prContextPage.evaluate(() => { + const secondViewer = document.querySelector("[data-pr-diff-viewer]"); + const spacer = document.createElement("div"); + spacer.style.height = "2400px"; + spacer.dataset.cachedSwitchSpacer = "second"; + secondViewer.append(spacer); + const secondDetail = secondViewer.closest(".pr-diff-detail"); + void secondDetail.scrollHeight; + secondDetail.scrollTop = 240; + secondViewer.dataset.cachedSwitchProbe = "second"; + window.__tychoCachedSecondViewer = secondViewer; + }); + await prContextPage.click(`.pr-diff-nav-item[href*='${encodeURIComponent(cachedSwitchIds.firstId)}']`); + await prContextPage.waitForSelector(`[data-pr-diff-viewer][data-pull-request-id='${cachedSwitchIds.firstId}']`, { state: "visible", timeout: 10_000 }); + const cachedFirstViewerContract = await prContextPage.evaluate(() => ({ + reused: document.querySelector("[data-pr-diff-viewer]") === window.__tychoCachedFirstViewer && + document.querySelector("[data-pr-diff-viewer]")?.dataset.cachedSwitchProbe === "first", + comment: document.querySelector("[data-pr-context-comment]")?.value, + selected: document.querySelectorAll("[data-select-pr-diff-line]:checked").length, + scrollTop: document.querySelector(".pr-diff-detail")?.scrollTop, + })); + const cachedFirstViewerReused = cachedFirstViewerContract.reused && + cachedFirstViewerContract.comment === "Persistent first PR comment" && + cachedFirstViewerContract.selected === 1 && + Math.abs(cachedFirstViewerContract.scrollTop - 140) <= 1; + await prContextPage.click(`.pr-diff-nav-item[href*='${encodeURIComponent(cachedSwitchIds.secondId)}']`); + await prContextPage.waitForSelector(`[data-pr-diff-viewer][data-pull-request-id='${cachedSwitchIds.secondId}']`, { state: "visible", timeout: 10_000 }); + const cachedSecondViewerContract = await prContextPage.evaluate(() => ({ + reused: document.querySelector("[data-pr-diff-viewer]") === window.__tychoCachedSecondViewer && + document.querySelector("[data-pr-diff-viewer]")?.dataset.cachedSwitchProbe === "second", + scrollTop: document.querySelector(".pr-diff-detail")?.scrollTop, + cacheKeys: Array.from(state.pullRequestDiffViewers.keys()), + currentKey: pollContentKey(document.querySelector("[data-pr-diff-viewer]")), + firstKey: pollContentKey(window.__tychoCachedFirstViewer), + secondKey: pollContentKey(window.__tychoCachedSecondViewer), + })); + const cachedSecondViewerReused = cachedSecondViewerContract.reused && + Math.abs(cachedSecondViewerContract.scrollTop - 240) <= 1; + if (!cachedFirstViewerReused || !cachedSecondViewerReused) { + throw new Error(`Cached PR navigation lost viewer, form, selection, or scroll state: ${JSON.stringify({ cachedFirstViewerContract, cachedSecondViewerContract })}`); + } + await prContextPage.evaluate(() => document.querySelector("#cached-pr-scroll-smoke-style")?.remove()); + await prContextPage.setViewportSize({ width: 390, height: 844 }); + const mobileScrollContract = await prContextPage.evaluate(async ({ firstId, secondId }) => { + const switchTo = async (id) => { + Array.from(document.querySelectorAll(".pr-diff-nav-item")) + .find((item) => item.getAttribute("href")?.includes(encodeURIComponent(id))) + .click(); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + }; + window.scrollTo(0, 500); + await new Promise((resolve) => requestAnimationFrame(resolve)); + const secondTop = window.scrollY; + await switchTo(firstId); + window.scrollTo(0, 300); + await new Promise((resolve) => requestAnimationFrame(resolve)); + const firstTop = window.scrollY; + await switchTo(secondId); + const restoredSecondTop = window.scrollY; + await switchTo(firstId); + return { firstTop, secondTop, restoredFirstTop: window.scrollY, restoredSecondTop }; + }, cachedSwitchIds); + await prContextPage.setViewportSize({ width: 1280, height: 900 }); + if (mobileScrollContract.firstTop < 1 || mobileScrollContract.secondTop < 1 || + Math.abs(mobileScrollContract.restoredFirstTop - mobileScrollContract.firstTop) > 1 || + Math.abs(mobileScrollContract.restoredSecondTop - mobileScrollContract.secondTop) > 1) { + throw new Error(`Cached mobile PR navigation lost page scroll: ${JSON.stringify(mobileScrollContract)}`); + } + await prContextPage.evaluate(({ key, id }) => { + const selection = pullRequestDiffSelection(key, id); + selection.anchor = null; + selection.lines = []; + selection.comment = ""; + state.renderedViewHtml = ""; + render(); + }, { key: prContextAgentKey, id: cachedSwitchIds.firstId }); + await prContextPage.waitForSelector(`[data-pr-diff-viewer][data-pull-request-id='${cachedSwitchIds.firstId}']`, { state: "visible", timeout: 10_000 }); const selectableLines = prContextPage.locator("[data-select-pr-diff-line]"); if (await selectableLines.count() !== 2) throw new Error("PR detail did not expose selectable changed lines"); const firstLine = selectableLines.nth(0); @@ -1892,7 +2217,7 @@ def write_smoke_script(path) const originalLines = diff.files[0].hunks[0].lines; const originalSnapshotId = diff.snapshot_id; const originalExpandAll = state.prDiffExpandAll[key]; - diff.files[0].hunks[0].lines = Array.from({ length: 1_500 }, (_, index) => ({ + diff.files[0].hunks[0].lines = Array.from({ length: 5_000 }, (_, index) => ({ kind: "context", old_number: index + 1, new_number: index + 1, content: `large diff line ${index + 1}`, })); state.renderedViewHtml = ""; @@ -2025,22 +2350,20 @@ def write_smoke_script(path) !largeDiffSelectionContract.steadyPollViewerPersistent || largeDiffSelectionContract.steadyPollDiffLineRenderCount !== 0 || largeDiffSelectionContract.steadyPollComment !== "Live comment text must not invalidate the diff shell." || - largeDiffSelectionContract.steadyPollRenderSyncMs >= largeDiffSelectionContract.fullRenderSyncMs * 0.5 || largeDiffSelectionContract.forcedRenderDiffLineCount !== 0 || !largeDiffSelectionContract.forcedViewerPersistent || !largeDiffSelectionContract.forcedCommentPersistent || !largeDiffSelectionContract.forcedTitleUpdated || - largeDiffSelectionContract.changedSnapshotRenderCount !== 1_500 || + largeDiffSelectionContract.changedSnapshotRenderCount !== 5_000 || !largeDiffSelectionContract.changedSnapshotViewerReplaced || - largeDiffSelectionContract.collapsedRenderCount !== 1_500 || + largeDiffSelectionContract.collapsedRenderCount !== 5_000 || !largeDiffSelectionContract.collapsedViewerReplaced || !largeDiffSelectionContract.collapsedFilesClosed || largeDiffSelectionContract.scrollDelta > 1 || largeDiffSelectionContract.lineDelta > 1 || - largeDiffSelectionContract.pollScrollDelta > 1 || largeDiffSelectionContract.pollLineDelta > 1 || - largeDiffSelectionContract.durationMs > 500) { + largeDiffSelectionContract.pollScrollDelta > 1 || largeDiffSelectionContract.pollLineDelta > 1) { throw new Error(`Large PR selection did not preserve the diff, form, or scroll: ${JSON.stringify(largeDiffSelectionContract)}`); } - console.log(`PR rendering (1,500 lines): full ${largeDiffSelectionContract.fullRenderSyncMs.toFixed(1)}ms sync / selection ${largeDiffSelectionContract.selectionSyncMs.toFixed(1)}ms sync / first poll ${largeDiffSelectionContract.pollRenderSyncMs.toFixed(1)}ms sync / steady poll ${largeDiffSelectionContract.steadyPollRenderSyncMs.toFixed(1)}ms sync`); + console.log(`PR rendering (5,000 lines): full ${largeDiffSelectionContract.fullRenderSyncMs.toFixed(1)}ms sync / selection ${largeDiffSelectionContract.selectionSyncMs.toFixed(1)}ms sync / first poll ${largeDiffSelectionContract.pollRenderSyncMs.toFixed(1)}ms sync / steady poll ${largeDiffSelectionContract.steadyPollRenderSyncMs.toFixed(1)}ms sync`); await prContextPage.evaluate((key) => { const route = parseRoute(); const pullRequestId = selectedPullRequestId(key, route.pullRequestId); diff --git a/docs/PROJECT_STATUS.md b/docs/PROJECT_STATUS.md index 730ba93..9632c57 100644 --- a/docs/PROJECT_STATUS.md +++ b/docs/PROJECT_STATUS.md @@ -56,6 +56,7 @@ Key references: | Remote artifact rendering | Render attached HTML in an origin-isolated iframe with a restrictive content policy and package explicitly referenced, allowlisted workspace web assets; render sanitized Markdown Mermaid fences with a pinned, conditional CDN loader in strict mode | Interactive lessons and shared course assets remain usable without granting generated HTML access to Tycho or browser storage, and ordinary Markdown does not pay the Mermaid download cost | | Pull request review | Agent-scoped PR diff inspection remains available; the cross-agent Review Inbox is paused because its eager aggregation is too slow and unresponsive | Redesign inbox discovery and loading around bounded, incremental work before restoring its route; retain GitHub App and `gh` compatibility | | Agent pull request catalog | Persist canonical PR references and origin title/status metadata in an agent-owned `.pull_request_catalog.json` sidecar; keep ordinary agent PR listing network-free | Opening one agent reads only that agent's catalog; displayed titles and Open/Draft/Closed/Merged state come from cached GitHub metadata, refreshes remain explicit, and archiving moves the catalog with the agent | +| Agent pull request switching | Give foreground navigation a reserved request slot, cancel stale background work outside the open PR route, preload at most six saved snapshots, retain at most twelve payloads plus six visited viewers and per-PR scroll positions, and render lines in 100-line containment chunks | Persistent snapshots remain authoritative; immutable parsed-store reuse and bounded browser caches make warm switching independent of polling while preserving each PR's form, selection, and desktop/mobile scroll state | | Conversation block scrolling | Initial chat load bottom-aligns the latest block when it fits, oversized blocks start at row 1, and navigation scrolls only enough to reveal the selected block | The selected label/cursor must remain visible and predictable while keeping surrounding recent context on first open | | Conversation viewport offsets | Block `line_offset` / `line_height` are derived from the final rendered rows; long unbroken preview tokens are hard-wrapped before entering the viewport, and footer debug is computed after viewport sync | Bubbles `Viewport` counts newline-separated lines, while terminals visually wrap long tokens; stale or mismatched offsets cause misleading `visible 0/0` debug and cropped selected blocks | | Inquiry submission | Gated review step inside a rounded box | Prevents accidental structured submissions | diff --git a/docs/PULL_REQUEST_DIFFS.md b/docs/PULL_REQUEST_DIFFS.md index 970377a..fc5db4a 100644 --- a/docs/PULL_REQUEST_DIFFS.md +++ b/docs/PULL_REQUEST_DIFFS.md @@ -10,6 +10,8 @@ Review posting remains off by default. Operators must also set `TYCHO_GITHUB_WRI Agent-scoped PR discovery persists canonical references and compact metadata in an agent-owned `~/.tycho/logs/agents/.pull_request_catalog.json` sidecar. Opening an agent's PR list reads only that agent's catalog and does not issue one GitHub request per PR. The cached GitHub title replaces attachment-supplied display text, and the list/detail surfaces show Open, Draft, Closed, or Merged state. Existing saved snapshots seed missing catalog metadata without another patch fetch. **Refresh metadata** updates the catalog explicitly; fetching one or all diffs remains a separate patch operation. Archiving an agent moves its catalog, backup, and lock sidecars with its other logs. +The Remote UI preloads up to six saved diff snapshots for the open agent, cancels stale queued preloads when that route changes, retains at most twelve snapshot payloads, and keeps up to six visited diff viewers and scroll positions in bounded LRU caches. Foreground navigation has priority and one reserved request slot, so a click cannot wait behind background preloads; an explicit refresh that meets an in-flight preload follows it with the required refresh request. Single and bulk refresh results use the same bounded payload cache and protect the selected PR. Diff lines render in 100-line containment chunks so off-screen code does not force full-page layout. The persistent snapshot store remains authoritative, while the server reuses an immutable parsed document until the snapshot file's inode, size, or nanosecond mtime changes. Switching among visited PRs therefore avoids both JSON reparsing and rebuilding thousands of diff-line DOM controls; first visits use the preloaded snapshot and bounded layout. + ## Workflow ```mermaid diff --git a/lib/hq/domain/pull_request_diff.rb b/lib/hq/domain/pull_request_diff.rb index 0498ba7..189d299 100644 --- a/lib/hq/domain/pull_request_diff.rb +++ b/lib/hq/domain/pull_request_diff.rb @@ -48,10 +48,13 @@ class Store def initialize(path = File.join(HQ::AGENT_LOGS_DIR, "pull_request_diffs.json")) @path = path + @document_mutex = Mutex.new + @document_signature = nil + @document_cache = nil end def all - parsed = FileStore.read_json(@path, fallback: {}) + parsed = document return {} unless parsed.is_a?(Hash) return parsed.fetch("snapshots", {}) if parsed["version"] == STORE_VERSION @@ -62,7 +65,7 @@ def all end def fetch_snapshot(snapshot_id) - parsed = FileStore.read_json(@path, fallback: {}) + parsed = document return nil unless parsed.is_a?(Hash) && parsed["version"] == STORE_VERSION parsed.fetch("history", {})[snapshot_id.to_s] @@ -77,23 +80,59 @@ def fetch(id) def save(snapshot) with_lock do id = snapshot.fetch("id") - snapshots = all + snapshots = all.dup snapshots[id] = snapshot snapshots = snapshots.sort_by { |_key, value| value["fetched_at"].to_s }.last(MAX_SNAPSHOTS).to_h - parsed = FileStore.read_json(@path, fallback: {}) - history = parsed.is_a?(Hash) && parsed["version"] == STORE_VERSION ? parsed.fetch("history", {}) : {} + parsed = document + history = parsed.is_a?(Hash) && parsed["version"] == STORE_VERSION ? parsed.fetch("history", {}).dup : {} history[snapshot["snapshot_id"]] = snapshot if snapshot["snapshot_id"] history = history.sort_by { |_key, value| value["fetched_at"].to_s }.last(MAX_SNAPSHOTS).to_h FileStore.write_json( @path, { "version" => STORE_VERSION, "snapshots" => snapshots, "history" => history } ) + invalidate_document_cache end snapshot end private + def document + signature = document_signature + @document_mutex.synchronize do + return @document_cache if @document_cache && @document_signature == signature + + @document_cache = deep_freeze(FileStore.read_json(@path, fallback: {})) + @document_signature = signature + @document_cache + end + end + + def deep_freeze(value) + case value + when Hash + value.each { |key, item| deep_freeze(key); deep_freeze(item) } + when Array + value.each { |item| deep_freeze(item) } + end + value.freeze + end + + def document_signature + stat = File.stat(@path) + [stat.ino, stat.size, stat.mtime.to_i, stat.mtime.nsec] + rescue Errno::ENOENT + nil + end + + def invalidate_document_cache + @document_mutex.synchronize do + @document_signature = nil + @document_cache = nil + end + end + def with_lock FileUtils.mkdir_p(File.dirname(@path)) File.open("#{@path}.lock", File::RDWR | File::CREAT, 0o600) do |lock| diff --git a/lib/hq/remote_ui/assets/app.css b/lib/hq/remote_ui/assets/app.css index 811b8e5..eef05a8 100644 --- a/lib/hq/remote_ui/assets/app.css +++ b/lib/hq/remote_ui/assets/app.css @@ -3430,6 +3430,12 @@ select { line-height: 1.45; } +.diff-line-chunk { + display: grid; + content-visibility: auto; + contain-intrinsic-size: auto 2000px; +} + .diff-line { position: relative; display: grid; diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index 11314ad..eed6672 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -7,6 +7,12 @@ const DEFAULT_REFRESH_INTERVALS = { }; const FORM_POLL_QUIET_MS = 3_000; const MAX_CONCURRENT_PR_DIFF_FETCHES = 3; +const MAX_PRELOADED_PR_DIFFS_PER_AGENT = 6; +const MAX_CACHED_PR_DIFF_DATA = 12; +const MAX_CACHED_PR_DIFF_VIEWERS = 6; +const MAX_CACHED_PR_DIFF_SCROLL_POSITIONS = 6; +const MAX_PR_DIFF_PRELOAD_FAILURES = 12; +const DIFF_LINE_RENDER_CHUNK_SIZE = 100; const PROMPT_ATTACHMENT_LIMITS = { maxFiles: 5, maxBytes: 10 * 1024 * 1024, @@ -580,6 +586,10 @@ const state = { projectWorkspacePreviewRequests: {}, pullRequests: {}, pullRequestDiffs: {}, + pullRequestDiffDataOrder: new Map(), + pullRequestDiffViewers: new Map(), + pullRequestDiffScrollPositions: new Map(), + pullRequestDiffPreloadFailures: new Set(), pullRequestInbox: null, pullRequestReviews: {}, githubLogin: null, @@ -1117,6 +1127,7 @@ function applyResourceCatalog(data) { Object.assign(detail, agent); }); const activeAgentKeys = new Set(state.agents.map((agent) => agent.key)); + prunePullRequestDiffState(activeAgentKeys); Object.keys(state.agentDetails).forEach((key) => { if (!activeAgentKeys.has(key)) delete state.agentDetails[key]; }); @@ -1758,6 +1769,7 @@ async function refresh(options = {}) { async function ensureRouteData(options = {}) { const route = parseRoute(); + cancelBackgroundPullRequestDiffFetchesExcept(route.type === "agentPullRequests" ? route.key : ""); let agentShellData = Promise.resolve(); if (route.type === "tab" && route.tab === "settings") { await Promise.all([ @@ -1787,7 +1799,11 @@ async function ensureRouteData(options = {}) { const refs = state.pullRequests[route.key]?.items || []; const selected = selectedPullRequestId(route.key, route.pullRequestId); const hasSnapshot = refs.find((item) => item.id === selected)?.snapshot; - if (selected && hasSnapshot) await ensurePullRequestDiff(route.key, selected, options.forcePullRequestDiff); + const selectedDiff = selected && hasSnapshot + ? ensurePullRequestDiff(route.key, selected, options.forcePullRequestDiff) + : Promise.resolve(); + preloadSavedPullRequestDiffs(route.key, refs, selected); + await selectedDiff; })(); await Promise.all([agentShellData, pullRequestData]); } @@ -2043,24 +2059,48 @@ async function ensurePullRequestReview(id, force = false) { async function ensurePullRequestDiff(agentKey, pullRequestId, force = false) { if (!agentKey || !pullRequestId) return; const key = pullRequestDiffKey(agentKey, pullRequestId); + touchPullRequestDiffData(key); if (!force && state.pullRequestDiffs[key] && !state.pullRequestDiffs[key].loading && !state.pullRequestDiffFetches[key]) return; await enqueuePullRequestDiffFetch(agentKey, pullRequestId, false, force); } +function preloadSavedPullRequestDiffs(agentKey, references, selectedId) { + const candidates = references + .filter((item) => item?.snapshot && item.id) + .slice(0, MAX_PRELOADED_PR_DIFFS_PER_AGENT); + candidates.forEach((item) => { + const key = pullRequestDiffKey(agentKey, item.id); + if (state.pullRequestDiffPreloadFailures.has(key)) return; + if (state.pullRequestDiffs[key] && !state.pullRequestDiffs[key].loading) return; + if (item.id === selectedId) return; + + void enqueuePullRequestDiffFetch(agentKey, item.id, false, false, { background: true }); + }); +} + function isPullRequestDiffFetching(agentKey, pullRequestId) { const key = pullRequestDiffKey(agentKey, pullRequestId); return Boolean(state.pullRequestDiffFetches[key]); } -function enqueuePullRequestDiffFetch(agentKey, pullRequestId, refresh = false, force = false) { +function enqueuePullRequestDiffFetch(agentKey, pullRequestId, refresh = false, force = false, options = {}) { const key = pullRequestDiffKey(agentKey, pullRequestId); if (!agentKey || !pullRequestId) return Promise.resolve(); + if (!options.background) state.pullRequestDiffPreloadFailures.delete(key); const existing = state.pullRequestDiffFetches[key]; if (existing?.queued || existing?.inFlight) { + if (existing.inFlight && refresh && !options.background) { + return existing.promise.then(() => enqueuePullRequestDiffFetch(agentKey, pullRequestId, true, force)); + } if (force) existing.force = true; if (refresh) existing.refresh = true; + if (!options.background) { + existing.background = false; + existing.discard = false; + } + drainPullRequestDiffFetchQueue(); return existing.promise; } @@ -2071,6 +2111,7 @@ function enqueuePullRequestDiffFetch(agentKey, pullRequestId, refresh = false, f pullRequestId, refresh, force, + background: options.background === true, queued: true, inFlight: false, }; @@ -2087,8 +2128,13 @@ function drainPullRequestDiffFetchQueue() { if (!state.pullRequestDiffFetchQueue.length) return; while (state.pullRequestDiffsInFlight < MAX_CONCURRENT_PR_DIFF_FETCHES && state.pullRequestDiffFetchQueue.length > 0) { - const next = state.pullRequestDiffFetchQueue.shift(); - if (!next) continue; + const foregroundIndex = state.pullRequestDiffFetchQueue.findIndex((request) => !request.background); + const next = foregroundIndex >= 0 + ? state.pullRequestDiffFetchQueue.splice(foregroundIndex, 1)[0] + : state.pullRequestDiffsInFlight < MAX_CONCURRENT_PR_DIFF_FETCHES - 1 + ? state.pullRequestDiffFetchQueue.shift() + : null; + if (!next) break; void processPullRequestDiffFetch(next); } @@ -2115,28 +2161,111 @@ async function processPullRequestDiffFetch(fetchRequest) { request.queued = false; request.inFlight = true; state.pullRequestDiffsInFlight += 1; - render(); + if (!request.background) render(); + let backgroundFailed = false; try { const route = `/agents/${encodeURIComponent(agentKey)}/pull-requests/${encodeURIComponent(pullRequestId)}/`; const data = request.refresh ? await apiPost(`${route}refresh`) : await apiGet(`${route}diff`); - state.pullRequestDiffs[key] = data.diff || { id: pullRequestId, files: [] }; + if (!request.discard) { + state.pullRequestDiffs[key] = data.diff || { id: pullRequestId, files: [] }; + rememberPullRequestDiffData(key); + } } catch (error) { - state.pullRequestDiffs[key] = { error: error.message, files: [] }; + if (request.discard) { + delete state.pullRequestDiffs[key]; + } else if (request.background) { + backgroundFailed = true; + rememberPullRequestDiffPreloadFailure(key); + delete state.pullRequestDiffs[key]; + } else { + state.pullRequestDiffs[key] = { error: error.message, files: [] }; + } } finally { state.pullRequestDiffsInFlight -= 1; - state.pullRequestDiffs[key] = state.pullRequestDiffs[key] || { id: pullRequestId, files: [] }; - state.pullRequestDiffs[key].loading = false; + if (request.discard) { + delete state.pullRequestDiffs[key]; + } else if (!backgroundFailed) { + state.pullRequestDiffs[key] = state.pullRequestDiffs[key] || { id: pullRequestId, files: [] }; + state.pullRequestDiffs[key].loading = false; + } const completed = state.pullRequestDiffFetches[key]; delete state.pullRequestDiffFetches[key]; - if (completed?.resolve) completed.resolve(state.pullRequestDiffs[key]); - render(); + if (completed?.resolve) completed.resolve(state.pullRequestDiffs[key] || null); + if (!completed?.background) render(); drainPullRequestDiffFetchQueue(); } } +function touchPullRequestDiffData(key) { + if (!state.pullRequestDiffDataOrder.has(key)) return; + state.pullRequestDiffDataOrder.delete(key); + state.pullRequestDiffDataOrder.set(key, true); +} + +function rememberPullRequestDiffData(key) { + state.pullRequestDiffDataOrder.delete(key); + state.pullRequestDiffDataOrder.set(key, true); + while (state.pullRequestDiffDataOrder.size > MAX_CACHED_PR_DIFF_DATA) { + const oldest = state.pullRequestDiffDataOrder.keys().next().value; + state.pullRequestDiffDataOrder.delete(oldest); + delete state.pullRequestDiffs[oldest]; + } +} + +function rememberPullRequestDiffPreloadFailure(key) { + state.pullRequestDiffPreloadFailures.delete(key); + state.pullRequestDiffPreloadFailures.add(key); + while (state.pullRequestDiffPreloadFailures.size > MAX_PR_DIFF_PRELOAD_FAILURES) { + state.pullRequestDiffPreloadFailures.delete(state.pullRequestDiffPreloadFailures.values().next().value); + } +} + +function cancelBackgroundPullRequestDiffFetchesExcept(agentKey) { + Object.values(state.pullRequestDiffFetches).forEach((request) => { + if (!request.background || request.agentKey === agentKey) return; + if (request.inFlight) { + request.discard = true; + return; + } + + state.pullRequestDiffFetchQueue = state.pullRequestDiffFetchQueue.filter((queued) => queued !== request); + delete state.pullRequestDiffFetches[request.key]; + delete state.pullRequestDiffs[request.key]; + request.resolve?.(null); + }); + drainPullRequestDiffFetchQueue(); +} + +function prunePullRequestDiffState(activeAgentKeys) { + const active = (key) => Array.from(activeAgentKeys).some((agentKey) => ( + String(key).startsWith(`${agentKey}:`) || String(key).startsWith(`pr-diff-viewer:${agentKey}:`) + )); + Object.keys(state.pullRequestDiffs).forEach((key) => { + if (!active(key)) delete state.pullRequestDiffs[key]; + }); + [state.pullRequestDiffDataOrder, state.pullRequestDiffViewers, state.pullRequestDiffScrollPositions].forEach((cache) => { + Array.from(cache.keys()).forEach((key) => { + if (!active(key)) cache.delete(key); + }); + }); + Array.from(state.pullRequestDiffPreloadFailures).forEach((key) => { + if (!active(key)) state.pullRequestDiffPreloadFailures.delete(key); + }); + Object.values(state.pullRequestDiffFetches).forEach((request) => { + if (active(request.key)) return; + if (request.inFlight) { + request.discard = true; + return; + } + state.pullRequestDiffFetchQueue = state.pullRequestDiffFetchQueue.filter((queued) => queued !== request); + delete state.pullRequestDiffFetches[request.key]; + request.resolve?.(null); + }); +} + async function ensureConversation(agent, force = false) { const cached = state.conversations[agent.key]; if (cached?.loading) return; @@ -2369,6 +2498,7 @@ function renderCurrentView() { function replaceView(html) { const routeKey = routeStateKey(parseRoute()); const sameRoute = state.renderedRouteKey === routeKey; + const reusePullRequestDiffViewer = Boolean(state.renderedViewHtml); if (sameRoute && state.renderedViewHtml === html) { syncPendingForms(); syncViewControls(); @@ -2391,6 +2521,7 @@ function replaceView(html) { if (!reconciled && state.speechRecognition) stopSpeechMode({ immediate: true }); if (!reconciled) replaceViewContent(html); transplantPreservedPollContent(els.view, preservedPollContent); + if (reusePullRequestDiffViewer) transplantPreservedPollContent(els.view, state.pullRequestDiffViewers); state.renderedRouteKey = routeKey; state.renderedViewHtml = html; syncMarkdownHeadingAnchors(); @@ -2400,6 +2531,7 @@ function replaceView(html) { syncViewControls(); syncFullScreenComposerModal(); syncAgentDockLayout(); + restoreVisiblePullRequestDiffScroll(); queueMermaidRendering(); } @@ -5323,6 +5455,7 @@ function renderAgentAttachmentView(agent, attachmentId, options = {}) { } function renderAgentPullRequestDiffView(agent, pullRequestId, options = {}) { + cacheVisiblePullRequestDiffViewer(); const data = state.pullRequests[agent.key] || { loading: true, items: [] }; const refs = Array.isArray(data.items) ? data.items : []; const selectedId = selectedPullRequestId(agent.key, pullRequestId); @@ -5448,7 +5581,7 @@ function renderPullRequestDiffDetail(agent, item, diff, options = {}) { const selection = pullRequestDiffSelection(agent.key, item.id, diff.snapshot_id || ""); const viewerStateKey = `pr-diff-viewer:${agent.key}:${item.id}`; const viewerVersion = `${diff.snapshot_id || "none"}:${expandAll ? "expanded" : "collapsed"}`; - const viewerBody = reusablePullRequestDiffViewer(agent.key, item.id, viewerStateKey, viewerVersion) + const viewerBody = options.reuseViewer !== false && reusablePullRequestDiffViewer(agent.key, item.id, viewerStateKey, viewerVersion) ? "" : files.length ? files.map((file, index) => renderProjectDiffFile(file, index, { @@ -5481,11 +5614,67 @@ function renderPullRequestDiffDetail(agent, item, diff, options = {}) { } function reusablePullRequestDiffViewer(agentKey, pullRequestId, stateKey, version) { - if (!state.renderedViewHtml || state.renderedRouteKey !== routeStateKey(parseRoute())) return null; - const viewer = document.querySelector( - `[data-pr-diff-viewer][data-agent-key="${CSS.escape(agentKey)}"][data-pull-request-id="${CSS.escape(pullRequestId)}"]` - ); - return viewer && pollContentKey(viewer) === `${stateKey}:${version}` ? viewer : null; + const key = `${stateKey}:${version}`; + if (state.renderedViewHtml && state.renderedRouteKey === routeStateKey(parseRoute())) { + const viewer = document.querySelector( + `[data-pr-diff-viewer][data-agent-key="${CSS.escape(agentKey)}"][data-pull-request-id="${CSS.escape(pullRequestId)}"]` + ); + if (viewer && pollContentKey(viewer) === key) return viewer; + } + if (!state.renderedViewHtml) return null; + const cached = state.pullRequestDiffViewers.get(key); + if (!cached) return null; + + state.pullRequestDiffViewers.delete(key); + state.pullRequestDiffViewers.set(key, cached); + return cached; +} + +function cacheVisiblePullRequestDiffViewer() { + if (!state.renderedViewHtml) return; + const viewer = els.view.querySelector("[data-pr-diff-viewer]"); + cachePullRequestDiffScroll(viewer); + cachePullRequestDiffViewer(viewer); +} + +function cachePullRequestDiffScroll(viewer) { + const agentKey = viewer?.dataset.agentKey; + const pullRequestId = viewer?.dataset.pullRequestId; + if (!agentKey || !pullRequestId) return; + + const key = pullRequestDiffKey(agentKey, pullRequestId); + const detail = viewer.closest(".pr-diff-detail"); + state.pullRequestDiffScrollPositions.delete(key); + state.pullRequestDiffScrollPositions.set(key, { + detailTop: Math.max(0, detail?.scrollTop || 0), + pageTop: Math.max(0, window.scrollY || document.documentElement.scrollTop || 0), + }); + while (state.pullRequestDiffScrollPositions.size > MAX_CACHED_PR_DIFF_SCROLL_POSITIONS) { + state.pullRequestDiffScrollPositions.delete(state.pullRequestDiffScrollPositions.keys().next().value); + } +} + +function restoreVisiblePullRequestDiffScroll() { + const viewer = els.view.querySelector("[data-pr-diff-viewer]"); + const key = viewer ? pullRequestDiffKey(viewer.dataset.agentKey, viewer.dataset.pullRequestId) : ""; + const position = state.pullRequestDiffScrollPositions.get(key); + if (!position) return; + + const detail = viewer.closest(".pr-diff-detail"); + if (detail) detail.scrollTop = position.detailTop; + window.scrollTo({ top: position.pageTop, left: window.scrollX }); +} + +function cachePullRequestDiffViewer(viewer) { + const key = pollContentKey(viewer); + if (!key) return; + + state.pullRequestDiffViewers.delete(key); + state.pullRequestDiffViewers.set(key, viewer); + while (state.pullRequestDiffViewers.size > MAX_CACHED_PR_DIFF_VIEWERS) { + const oldest = state.pullRequestDiffViewers.keys().next().value; + state.pullRequestDiffViewers.delete(oldest); + } } function pullRequestDiffRenderContext(agent, item, diff, selection) { @@ -6968,14 +7157,21 @@ function renderDiffHunk(hunk, index, options = {}) { const finalSelectedLineIndex = selectedLines.length ? Math.max(...selectedLines.map((line) => Number(line.line_index))) : -1; - const renderedLines = lines.map((line, lineIndex) => ` - ${renderDiffLine(line, lineIndex, { ...options, hunkIndex: index })} - ${lineIndex === finalSelectedLineIndex ? renderPullRequestInlineComment(options.pullRequestContext, options.path, index) : ""} - `).join(""); + const renderedLines = []; + for (let start = 0; start < lines.length; start += DIFF_LINE_RENDER_CHUNK_SIZE) { + const chunk = lines.slice(start, start + DIFF_LINE_RENDER_CHUNK_SIZE).map((line, chunkIndex) => { + const lineIndex = start + chunkIndex; + return ` + ${renderDiffLine(line, lineIndex, { ...options, hunkIndex: index })} + ${lineIndex === finalSelectedLineIndex ? renderPullRequestInlineComment(options.pullRequestContext, options.path, index) : ""} + `; + }).join(""); + renderedLines.push(`
${chunk}
`); + } return `
${escapeHtml(hunk.header || "@@")}
-
${renderedLines}
+
${renderedLines.join("")}
`; } @@ -13974,11 +14170,25 @@ async function refreshAllPullRequestDiffs(agentKey) { try { setConnection("Fetching PR diffs"); const data = await apiPost(`/agents/${encodeURIComponent(agentKey)}/pull-requests/refresh`); - (data.refreshed || []).forEach((diff) => { + const route = parseRoute(); + const selectedId = route.type === "agentPullRequests" && route.key === agentKey + ? selectedPullRequestId(agentKey, route.pullRequestId) + : ""; + const selectedKey = selectedId ? pullRequestDiffKey(agentKey, selectedId) : ""; + const selectedDiff = selectedKey ? state.pullRequestDiffs[selectedKey] : null; + const refreshed = Array.isArray(data.refreshed) ? data.refreshed : []; + refreshed.sort((left, right) => Number(left?.id === selectedId) - Number(right?.id === selectedId)); + refreshed.forEach((diff) => { if (!diff?.id) return; - state.pullRequestDiffs[pullRequestDiffKey(agentKey, diff.id)] = diff; + const key = pullRequestDiffKey(agentKey, diff.id); + state.pullRequestDiffs[key] = diff; + rememberPullRequestDiffData(key); }); + if (selectedDiff && !refreshed.some((diff) => diff?.id === selectedId)) { + state.pullRequestDiffs[selectedKey] = selectedDiff; + rememberPullRequestDiffData(selectedKey); + } await ensureAgentPullRequests(agentKey, true); const failures = Array.isArray(data.failed) ? data.failed.length : 0; setConnection(failures ? `PR diffs refreshed with ${failures} failure${failures === 1 ? "" : "s"}` : "PR diffs refreshed"); diff --git a/test/pull_request_diff_test.rb b/test/pull_request_diff_test.rb index be6cb30..f4c9b11 100644 --- a/test/pull_request_diff_test.rb +++ b/test/pull_request_diff_test.rb @@ -45,6 +45,7 @@ def run! assert_freshness_separates_code_and_activity assert_snapshot_identity_omits_agent_key assert_store_preserves_concurrent_snapshots + assert_store_reuses_and_invalidates_document_cache puts "pull_request_diff_test: ok" end @@ -135,6 +136,26 @@ def assert_store_preserves_concurrent_snapshots end end + def assert_store_reuses_and_invalidates_document_cache + Dir.mktmpdir("tycho-pr-diff-cache") do |dir| + path = File.join(dir, "snapshots.json") + store = HQ::PullRequestDiff::Store.new(path) + store.save("id" => "first", "snapshot_id" => "first-snapshot") + first_read = store.all + assert(store.all.equal?(first_read), "expected unchanged snapshot files to reuse the parsed document") + assert(first_read.frozen? && first_read.fetch("first").frozen?, "expected cached snapshots to be immutable") + begin + first_read.fetch("first")["snapshot_id"] = "mutated" + rescue FrozenError + nil + end + assert(store.fetch("first")["snapshot_id"] == "first-snapshot", "expected callers not to mutate cached snapshots") + + HQ::PullRequestDiff::Store.new(path).save("id" => "second", "snapshot_id" => "second-snapshot") + assert(store.all.keys.sort == %w[first second], "expected external snapshot writes to invalidate the parsed document") + end + end + def reference_for(agent_key: nil) HQ::PullRequestDiff::Reference.new( id: HQ::PullRequestDiff.reference_id("github", "example/web", 123), From 0151ca98d620c6d4bb162270ec3ba3f33eafb85f Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Mon, 10 Aug 2026 07:57:15 +0700 Subject: [PATCH 09/12] Defer PR diff rendering during navigation --- bin/remote-ui-smoke | 115 +++++++++++++++++++++++++++++++-- docs/PROJECT_STATUS.md | 3 +- docs/PULL_REQUEST_DIFFS.md | 2 +- lib/hq/domain/managed_agent.rb | 7 +- lib/hq/remote_ui/assets/app.js | 105 +++++++++++++++++++++++++++--- test/managed_agent_test.rb | 38 +++++++++++ 6 files changed, 251 insertions(+), 19 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index 4be4553..669ab41 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -1748,6 +1748,62 @@ def write_smoke_script(path) ))) { throw new Error(`PR title or status did not reflect origin metadata: ${JSON.stringify(prOriginMetadata)}`); } + const asynchronousDiffRenderContract = await prContextPage.evaluate(async () => { + const originalRenderDiffLine = renderDiffLine; + let renderCount = 0; + renderDiffLine = (...args) => { + renderCount += 1; + return originalRenderDiffLine(...args); + }; + resetPullRequestDiffRender(); + state.renderedViewHtml = ""; + render(); + const immediateRenderCount = renderCount; + const loadingBeforePaint = Boolean(document.querySelector(".pr-diff-loading-state")); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + const deferredRenderCount = renderCount - immediateRenderCount; + renderDiffLine = originalRenderDiffLine; + return { immediateRenderCount, deferredRenderCount, loadingBeforePaint }; + }); + if (asynchronousDiffRenderContract.immediateRenderCount !== 0 || + asynchronousDiffRenderContract.deferredRenderCount < 1 || + !asynchronousDiffRenderContract.loadingBeforePaint) { + throw new Error(`PR diff rendered in the route-switch frame: ${JSON.stringify(asynchronousDiffRenderContract)}`); + } + const refreshedSnapshotRenderContract = await prContextPage.evaluate(async (key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diffKey = pullRequestDiffKey(key, pullRequestId); + const originalDiff = state.pullRequestDiffs[diffKey]; + const originalRenderDiffLine = renderDiffLine; + let renderCount = 0; + renderDiffLine = (...args) => { + renderCount += 1; + return originalRenderDiffLine(...args); + }; + state.pullRequestDiffs[diffKey] = { + ...state.pullRequestDiffs[diffKey], + snapshot_id: `${state.pullRequestDiffs[diffKey].snapshot_id}-refreshed`, + }; + state.renderedViewHtml = ""; + render(); + const immediateRenderCount = renderCount; + const loadingBeforePaint = Boolean(document.querySelector(".pr-diff-loading-state")); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + const deferredRenderCount = renderCount - immediateRenderCount; + renderDiffLine = originalRenderDiffLine; + state.pullRequestDiffs[diffKey] = originalDiff; + resetPullRequestDiffRender(); + state.renderedViewHtml = ""; + render(); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + return { immediateRenderCount, deferredRenderCount, loadingBeforePaint }; + }, prContextAgentKey); + if (refreshedSnapshotRenderContract.immediateRenderCount !== 0 || + refreshedSnapshotRenderContract.deferredRenderCount < 1 || + !refreshedSnapshotRenderContract.loadingBeforePaint) { + throw new Error(`Refreshed PR diff rendered before the first paint: ${JSON.stringify(refreshedSnapshotRenderContract)}`); + } const delayedConversationPattern = `**/agents/${encodeURIComponent(prContextAgentKey)}/conversation`; const originalPullRequestHash = await prContextPage.evaluate(() => location.hash); let releaseDelayedConversation; @@ -2104,12 +2160,21 @@ def write_smoke_script(path) } await prContextPage.evaluate(() => document.querySelector("#cached-pr-scroll-smoke-style")?.remove()); await prContextPage.setViewportSize({ width: 390, height: 844 }); + await prContextPage.evaluate(() => new Promise((resolve) => ( + requestAnimationFrame(() => requestAnimationFrame(resolve)) + ))); const mobileScrollContract = await prContextPage.evaluate(async ({ firstId, secondId }) => { const switchTo = async (id) => { Array.from(document.querySelectorAll(".pr-diff-nav-item")) .find((item) => item.getAttribute("href")?.includes(encodeURIComponent(id))) .click(); - await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); + const deadline = Date.now() + 10_000; + const renderKeyPrefix = `${pullRequestDiffKey(parseRoute().key, id)}:`; + while (!state.pullRequestDiffRenderReadyKey.startsWith(renderKeyPrefix)) { + if (Date.now() > deadline) throw new Error(`Timed out rendering ${id}`); + await new Promise((resolve) => setTimeout(resolve, 10)); + } + await new Promise((resolve) => requestAnimationFrame(resolve)); }; window.scrollTo(0, 500); await new Promise((resolve) => requestAnimationFrame(resolve)); @@ -2310,6 +2375,8 @@ def write_smoke_script(path) }; diff.snapshot_id = `${originalSnapshotId}-changed`; render({ preserveLiveEditor: true, preservePollContent: true }); + result.changedSnapshotImmediateRenderCount = changedSnapshotRenderCount; + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); renderDiffLine = originalRenderDiffLine; result.changedSnapshotRenderCount = changedSnapshotRenderCount; result.changedSnapshotViewerReplaced = pollViewer !== document.querySelector("[data-pr-diff-viewer]"); @@ -2322,9 +2389,13 @@ def write_smoke_script(path) return originalRenderDiffLine(...args); }; document.querySelector(`[data-toggle-pr-diff-expand-all="${CSS.escape(key)}"]`).click(); + result.collapsedImmediateRenderCount = collapsedRenderCount; + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); renderDiffLine = originalRenderDiffLine; result.collapsedRenderCount = collapsedRenderCount; result.collapsedViewerReplaced = changedSnapshotViewer !== document.querySelector("[data-pr-diff-viewer]"); + result.collapsedViewerPresent = Boolean(document.querySelector("[data-pr-diff-viewer]")); + result.collapsedLoadingVisible = Boolean(document.querySelector(".pr-diff-loading-state")); result.collapsedFilesClosed = Array.from(document.querySelectorAll("[data-pr-diff-viewer] .diff-file")) .every((detail) => !detail.open); @@ -2354,17 +2425,21 @@ def write_smoke_script(path) !largeDiffSelectionContract.forcedViewerPersistent || !largeDiffSelectionContract.forcedCommentPersistent || !largeDiffSelectionContract.forcedTitleUpdated || + largeDiffSelectionContract.changedSnapshotImmediateRenderCount !== 0 || largeDiffSelectionContract.changedSnapshotRenderCount !== 5_000 || !largeDiffSelectionContract.changedSnapshotViewerReplaced || - largeDiffSelectionContract.collapsedRenderCount !== 5_000 || + largeDiffSelectionContract.collapsedImmediateRenderCount !== 0 || + ![0, 5_000].includes(largeDiffSelectionContract.collapsedRenderCount) || !largeDiffSelectionContract.collapsedViewerReplaced || + !largeDiffSelectionContract.collapsedViewerPresent || + largeDiffSelectionContract.collapsedLoadingVisible || !largeDiffSelectionContract.collapsedFilesClosed || largeDiffSelectionContract.scrollDelta > 1 || largeDiffSelectionContract.lineDelta > 1 || largeDiffSelectionContract.pollScrollDelta > 1 || largeDiffSelectionContract.pollLineDelta > 1) { throw new Error(`Large PR selection did not preserve the diff, form, or scroll: ${JSON.stringify(largeDiffSelectionContract)}`); } console.log(`PR rendering (5,000 lines): full ${largeDiffSelectionContract.fullRenderSyncMs.toFixed(1)}ms sync / selection ${largeDiffSelectionContract.selectionSyncMs.toFixed(1)}ms sync / first poll ${largeDiffSelectionContract.pollRenderSyncMs.toFixed(1)}ms sync / steady poll ${largeDiffSelectionContract.steadyPollRenderSyncMs.toFixed(1)}ms sync`); - await prContextPage.evaluate((key) => { + await prContextPage.evaluate(async (key) => { const route = parseRoute(); const pullRequestId = selectedPullRequestId(key, route.pullRequestId); const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; @@ -2372,13 +2447,33 @@ def write_smoke_script(path) diff.files[0].hunks[0].lines.push({ kind: "context", old_number: 2, new_number: 2, content: "following unselected line", }); + state.pullRequestDiffViewers.clear(); + resetPullRequestDiffRender(); state.renderedViewHtml = ""; render(); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); }, prContextAgentKey); + await prContextPage.waitForSelector("[data-select-pr-diff-line]", { state: "visible", timeout: 10_000 }); await firstLine.click(); await secondLine.click({ modifiers: ["Shift"] }); await prContextPage.waitForSelector("[data-pr-inline-comment]", { state: "visible", timeout: 10_000 }); - await prContextPage.waitForFunction(() => document.querySelectorAll("[data-select-pr-diff-line]:checked").length === 2); + await prContextPage.evaluate(() => new Promise((resolve) => requestAnimationFrame(resolve))); + const rangeSelectionState = await prContextPage.evaluate((key) => { + const route = parseRoute(); + const pullRequestId = selectedPullRequestId(key, route.pullRequestId); + const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; + const selection = pullRequestDiffSelection(key, pullRequestId, diff.snapshot_id); + return { + checked: document.querySelectorAll("[data-select-pr-diff-line]:checked").length, + selected: selection.lines.map((line) => line.line_index), + anchor: selection.anchor?.line_index, + snapshotId: diff.snapshot_id, + domSnapshotIds: Array.from(document.querySelectorAll("[data-select-pr-diff-line]"), (line) => line.dataset.snapshotId).slice(0, 3), + }; + }, prContextAgentKey); + if (rangeSelectionState.checked !== 2) { + throw new Error(`PR line range did not select two lines: ${JSON.stringify(rangeSelectionState)}`); + } const selectedRange = await prContextPage.evaluate((key) => { const route = parseRoute(); const pullRequestId = selectedPullRequestId(key, route.pullRequestId); @@ -2453,9 +2548,13 @@ def write_smoke_script(path) cancelContract.checkedLines || cancelContract.selectedMarks || cancelContract.scrollDelta > 1 || cancelContract.lineDelta > 1) { throw new Error(`Cancel did not clear the persistent PR comment UI in place: ${JSON.stringify(cancelContract)}`); } + await prContextPage.evaluate(() => new Promise((resolve) => requestAnimationFrame(resolve))); await firstLine.click(); await secondLine.click({ modifiers: ["Shift"] }); - await prContextPage.waitForFunction(() => document.querySelectorAll("[data-select-pr-diff-line]:checked").length === 2); + await prContextPage.evaluate(() => new Promise((resolve) => requestAnimationFrame(resolve))); + if (await prContextPage.locator("[data-select-pr-diff-line]:checked").count() !== 2) { + throw new Error("PR range could not be reselected after clearing it"); + } await prContextPage.keyboard.press("Escape"); if (await prContextPage.locator("[data-select-pr-diff-line]:checked").count()) { const escapeState = await prContextPage.evaluate((key) => ({ @@ -2524,18 +2623,19 @@ def write_smoke_script(path) !(await prContextPage.locator("#growl").innerText()).includes("already attached")) { throw new Error("Duplicate PR context was not rejected visibly"); } - await prContextPage.evaluate((key) => { + await prContextPage.evaluate(async (key) => { const route = parseRoute(); const pullRequestId = selectedPullRequestId(key, route.pullRequestId); const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; diff.snapshot_id = "newer-snapshot"; state.renderedViewHtml = ""; render(); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); }, prContextAgentKey); if (!(await prContextPage.locator("[data-pending-pr-context]").innerText()).includes("outdated")) { throw new Error("Stale pending PR context is not called out before submission"); } - await prContextPage.evaluate((key) => { + await prContextPage.evaluate(async (key) => { const route = parseRoute(); const pullRequestId = selectedPullRequestId(key, route.pullRequestId); const diff = state.pullRequestDiffs[pullRequestDiffKey(key, pullRequestId)]; @@ -2544,6 +2644,7 @@ def write_smoke_script(path) : diff.snapshot_id; state.renderedViewHtml = ""; render(); + await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve))); }, prContextAgentKey); await prContextPage.setViewportSize({ width: 1280, height: 800 }); await prContextPage.evaluate(() => document.querySelector("#growl")?.classList.add("hidden")); diff --git a/docs/PROJECT_STATUS.md b/docs/PROJECT_STATUS.md index 9632c57..f962a9f 100644 --- a/docs/PROJECT_STATUS.md +++ b/docs/PROJECT_STATUS.md @@ -56,7 +56,8 @@ Key references: | Remote artifact rendering | Render attached HTML in an origin-isolated iframe with a restrictive content policy and package explicitly referenced, allowlisted workspace web assets; render sanitized Markdown Mermaid fences with a pinned, conditional CDN loader in strict mode | Interactive lessons and shared course assets remain usable without granting generated HTML access to Tycho or browser storage, and ordinary Markdown does not pay the Mermaid download cost | | Pull request review | Agent-scoped PR diff inspection remains available; the cross-agent Review Inbox is paused because its eager aggregation is too slow and unresponsive | Redesign inbox discovery and loading around bounded, incremental work before restoring its route; retain GitHub App and `gh` compatibility | | Agent pull request catalog | Persist canonical PR references and origin title/status metadata in an agent-owned `.pull_request_catalog.json` sidecar; keep ordinary agent PR listing network-free | Opening one agent reads only that agent's catalog; displayed titles and Open/Draft/Closed/Merged state come from cached GitHub metadata, refreshes remain explicit, and archiving moves the catalog with the agent | -| Agent pull request switching | Give foreground navigation a reserved request slot, cancel stale background work outside the open PR route, preload at most six saved snapshots, retain at most twelve payloads plus six visited viewers and per-PR scroll positions, and render lines in 100-line containment chunks | Persistent snapshots remain authoritative; immutable parsed-store reuse and bounded browser caches make warm switching independent of polling while preserving each PR's form, selection, and desktop/mobile scroll state | +| Agent pull request switching | Paint the route shell before asynchronously attaching its diff, give foreground navigation a reserved request slot, cancel stale background work outside the open PR route, preload at most six saved snapshots, retain at most twelve payloads plus six visited viewers and per-PR scroll positions, and render lines in 100-line containment chunks | Persistent snapshots remain authoritative; navigation never shares a frame with diff layout, while immutable parsed-store reuse and bounded browser caches preserve each PR's form, selection, and desktop/mobile scroll state | +| Managed-agent completion status | Let a validated structured result define Success, Partial, or Failed after a run; retain the process exit code as transport diagnostics | A usable structured result must not remain labeled Failed solely because its harness process exited nonzero, while missing or explicitly failed results still surface failure | | Conversation block scrolling | Initial chat load bottom-aligns the latest block when it fits, oversized blocks start at row 1, and navigation scrolls only enough to reveal the selected block | The selected label/cursor must remain visible and predictable while keeping surrounding recent context on first open | | Conversation viewport offsets | Block `line_offset` / `line_height` are derived from the final rendered rows; long unbroken preview tokens are hard-wrapped before entering the viewport, and footer debug is computed after viewport sync | Bubbles `Viewport` counts newline-separated lines, while terminals visually wrap long tokens; stale or mismatched offsets cause misleading `visible 0/0` debug and cropped selected blocks | | Inquiry submission | Gated review step inside a rounded box | Prevents accidental structured submissions | diff --git a/docs/PULL_REQUEST_DIFFS.md b/docs/PULL_REQUEST_DIFFS.md index fc5db4a..81bd8eb 100644 --- a/docs/PULL_REQUEST_DIFFS.md +++ b/docs/PULL_REQUEST_DIFFS.md @@ -10,7 +10,7 @@ Review posting remains off by default. Operators must also set `TYCHO_GITHUB_WRI Agent-scoped PR discovery persists canonical references and compact metadata in an agent-owned `~/.tycho/logs/agents/.pull_request_catalog.json` sidecar. Opening an agent's PR list reads only that agent's catalog and does not issue one GitHub request per PR. The cached GitHub title replaces attachment-supplied display text, and the list/detail surfaces show Open, Draft, Closed, or Merged state. Existing saved snapshots seed missing catalog metadata without another patch fetch. **Refresh metadata** updates the catalog explicitly; fetching one or all diffs remains a separate patch operation. Archiving an agent moves its catalog, backup, and lock sidecars with its other logs. -The Remote UI preloads up to six saved diff snapshots for the open agent, cancels stale queued preloads when that route changes, retains at most twelve snapshot payloads, and keeps up to six visited diff viewers and scroll positions in bounded LRU caches. Foreground navigation has priority and one reserved request slot, so a click cannot wait behind background preloads; an explicit refresh that meets an in-flight preload follows it with the required refresh request. Single and bulk refresh results use the same bounded payload cache and protect the selected PR. Diff lines render in 100-line containment chunks so off-screen code does not force full-page layout. The persistent snapshot store remains authoritative, while the server reuses an immutable parsed document until the snapshot file's inode, size, or nanosecond mtime changes. Switching among visited PRs therefore avoids both JSON reparsing and rebuilding thousands of diff-line DOM controls; first visits use the preloaded snapshot and bounded layout. +The Remote UI preloads up to six saved diff snapshots for the open agent, cancels stale queued preloads when that route changes, retains at most twelve snapshot payloads, and keeps up to six visited diff viewers and scroll positions in bounded LRU caches. Foreground navigation has priority and one reserved request slot, so a click cannot wait behind background preloads; an explicit refresh that meets an in-flight preload follows it with the required refresh request. Single and bulk refresh results use the same bounded payload cache and protect the selected PR. Route changes paint the PR shell and loading state first, then attach even an already-cached diff on a later animation frame, keeping diff layout out of the navigation frame. Diff lines render in 100-line containment chunks so off-screen code does not force full-page layout. The persistent snapshot store remains authoritative, while the server reuses an immutable parsed document until the snapshot file's inode, size, or nanosecond mtime changes. Switching among visited PRs therefore avoids both JSON reparsing and rebuilding thousands of diff-line DOM controls; first visits use the preloaded snapshot and bounded layout. ## Workflow diff --git a/lib/hq/domain/managed_agent.rb b/lib/hq/domain/managed_agent.rb index 1b43009..8da7481 100644 --- a/lib/hq/domain/managed_agent.rb +++ b/lib/hq/domain/managed_agent.rb @@ -597,9 +597,14 @@ def status return "blocked" if blocked? return "idle" if @started_at.nil? && last_run.nil? return "idle" if @last_exit_code.nil? - return "succeeded" if @last_exit_code.zero? return "stopped" if stopped_exit_code? + structured_status = @structured_result&.dig("status").to_s.strip + return "succeeded" if %w[success succeeded no_action_needed].include?(structured_status) + return "partial" if structured_status == "partial" + return "failed" if structured_status == "failed" + return "succeeded" if @last_exit_code.zero? + "failed" end diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index eed6672..e9fb53e 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -587,6 +587,10 @@ const state = { pullRequests: {}, pullRequestDiffs: {}, pullRequestDiffDataOrder: new Map(), + pullRequestDiffRenderReadyKey: "", + pullRequestDiffRenderFrame: null, + pullRequestDiffRenderPendingKey: "", + pullRequestDiffUserScrollGeneration: 0, pullRequestDiffViewers: new Map(), pullRequestDiffScrollPositions: new Map(), pullRequestDiffPreloadFailures: new Set(), @@ -1799,11 +1803,10 @@ async function ensureRouteData(options = {}) { const refs = state.pullRequests[route.key]?.items || []; const selected = selectedPullRequestId(route.key, route.pullRequestId); const hasSnapshot = refs.find((item) => item.id === selected)?.snapshot; - const selectedDiff = selected && hasSnapshot - ? ensurePullRequestDiff(route.key, selected, options.forcePullRequestDiff) - : Promise.resolve(); + if (selected && hasSnapshot) { + void ensurePullRequestDiff(route.key, selected, options.forcePullRequestDiff); + } preloadSavedPullRequestDiffs(route.key, refs, selected); - await selectedDiff; })(); await Promise.all([agentShellData, pullRequestData]); } @@ -5520,7 +5523,7 @@ function renderPullRequestDiffNavItem(agent, item, selectedId) { pullRequestFreshnessLabel(item), ].filter(Boolean).join(" / "); return ` - + ${escapeHtml(item.title || item.url || "Pull request")} @@ -5575,12 +5578,15 @@ function renderPullRequestDiffDetail(agent, item, diff, options = {}) { `}`; if (!diff || diff.loading || loading) return `${header}${tychoLoadingState("Loading PR diff", { className: "pr-diff-loading-state" })}`; if (diff.error) return `${header}${feedbackMessage("Diff unavailable", diff.error, { intent: "danger", announce: "assertive" })}`; + const viewerVersion = `${diff.snapshot_id || "none"}:${expandAll ? "expanded" : "collapsed"}`; + if (deferPullRequestDiffRender(agent.key, item.id, viewerVersion)) { + return `${header}${tychoLoadingState("Loading PR diff", { className: "pr-diff-loading-state" })}`; + } const files = Array.isArray(diff.files) ? diff.files : []; const summary = `${files.length} ${files.length === 1 ? "file" : "files"} / +${diff.additions || 0} -${diff.deletions || 0}`; const selection = pullRequestDiffSelection(agent.key, item.id, diff.snapshot_id || ""); const viewerStateKey = `pr-diff-viewer:${agent.key}:${item.id}`; - const viewerVersion = `${diff.snapshot_id || "none"}:${expandAll ? "expanded" : "collapsed"}`; const viewerBody = options.reuseViewer !== false && reusablePullRequestDiffViewer(agent.key, item.id, viewerStateKey, viewerVersion) ? "" : files.length @@ -5630,6 +5636,45 @@ function reusablePullRequestDiffViewer(agentKey, pullRequestId, stateKey, versio return cached; } +function deferPullRequestDiffRender(agentKey, pullRequestId, viewerVersion) { + const routeKey = pullRequestDiffKey(agentKey, pullRequestId); + const key = `${routeKey}:${viewerVersion}`; + if (state.pullRequestDiffRenderReadyKey === key) return false; + if (state.pullRequestDiffRenderFrame && state.pullRequestDiffRenderPendingKey === key) return true; + if (state.pullRequestDiffRenderFrame) window.cancelAnimationFrame(state.pullRequestDiffRenderFrame); + + state.pullRequestDiffRenderPendingKey = key; + state.pullRequestDiffRenderFrame = window.requestAnimationFrame(() => { + state.pullRequestDiffRenderFrame = window.requestAnimationFrame(() => { + state.pullRequestDiffRenderFrame = null; + state.pullRequestDiffRenderPendingKey = ""; + const route = parseRoute(); + const selectedId = route.type === "agentPullRequests" + ? selectedPullRequestId(route.key, route.pullRequestId) + : ""; + if (pullRequestDiffKey(route.key, selectedId) !== routeKey) return; + + const diff = state.pullRequestDiffs[routeKey]; + const expandAll = state.prDiffExpandAll[agentKey] !== false; + const currentVersion = `${diff?.snapshot_id || "none"}:${expandAll ? "expanded" : "collapsed"}`; + if (`${routeKey}:${currentVersion}` !== key) return; + + state.pullRequestDiffRenderReadyKey = key; + render(); + }); + }); + return true; +} + +function resetPullRequestDiffRender() { + state.pullRequestDiffRenderReadyKey = ""; + state.pullRequestDiffRenderPendingKey = ""; + if (!state.pullRequestDiffRenderFrame) return; + + window.cancelAnimationFrame(state.pullRequestDiffRenderFrame); + state.pullRequestDiffRenderFrame = null; +} + function cacheVisiblePullRequestDiffViewer() { if (!state.renderedViewHtml) return; const viewer = els.view.querySelector("[data-pr-diff-viewer]"); @@ -5641,6 +5686,10 @@ function cachePullRequestDiffScroll(viewer) { const agentKey = viewer?.dataset.agentKey; const pullRequestId = viewer?.dataset.pullRequestId; if (!agentKey || !pullRequestId) return; + const route = parseRoute(); + if (route.type === "agentPullRequests" && ( + route.key !== agentKey || selectedPullRequestId(route.key, route.pullRequestId) !== pullRequestId + )) return; const key = pullRequestDiffKey(agentKey, pullRequestId); const detail = viewer.closest(".pr-diff-detail"); @@ -5661,8 +5710,19 @@ function restoreVisiblePullRequestDiffScroll() { if (!position) return; const detail = viewer.closest(".pr-diff-detail"); + const userScrollGeneration = state.pullRequestDiffUserScrollGeneration; if (detail) detail.scrollTop = position.detailTop; window.scrollTo({ top: position.pageTop, left: window.scrollX }); + window.requestAnimationFrame(() => { + if (state.pullRequestDiffUserScrollGeneration !== userScrollGeneration) return; + + const current = els.view.querySelector("[data-pr-diff-viewer]"); + if (!current || pullRequestDiffKey(current.dataset.agentKey, current.dataset.pullRequestId) !== key) return; + + const currentDetail = current.closest(".pr-diff-detail"); + if (currentDetail) currentDetail.scrollTop = position.detailTop; + window.scrollTo({ top: position.pageTop, left: window.scrollX }); + }); } function cachePullRequestDiffViewer(viewer) { @@ -7355,9 +7415,12 @@ function syncPullRequestDiffSelection(agentKey, pullRequestId, options = {}) { restorePullRequestScrollPosition(scrollSnapshot); window.requestAnimationFrame(() => { // A native checkbox inside a label can finish its activation after the delegated click handler. - // Reassert the state model once without rebuilding the diff. - syncPullRequestDiffLineControls(controls, selectedKeys, selectionLocked); - if (options.focusComment && selection.lines.length) { + // Reassert the latest state model once without rebuilding the diff. A newer + // selection may have replaced this callback's snapshot in the same frame. + const currentSelection = pullRequestDiffSelection(agentKey, pullRequestId, diff.snapshot_id || ""); + const currentSelectedKeys = new Set(currentSelection.lines.map(pullRequestLineKey)); + syncPullRequestDiffLineControls(controls, currentSelectedKeys, state.pendingPullRequestCommentKeys.has(agentKey)); + if (options.focusComment && currentSelection.lines.length) { document.querySelector("[data-pr-context-comment]")?.focus({ preventScroll: true }); } restorePullRequestScrollPosition(scrollSnapshot); @@ -10920,6 +10983,13 @@ function onScroll() { window.requestAnimationFrame(handleScrollDirection); } +function notePullRequestDiffScrollIntent(event) { + if (!event.isTrusted || !els.view.querySelector("[data-pr-diff-viewer]")) return; + if (event.type === "keydown" && !["ArrowDown", "ArrowUp", "End", "Home", "PageDown", "PageUp", " "].includes(event.key)) return; + + state.pullRequestDiffUserScrollGeneration += 1; +} + function findAgent(key) { const listAgent = state.agents.find((agent) => agent.key === key) || null; const detailAgent = state.agentDetails[key] || @@ -12538,6 +12608,18 @@ els.authPanel.addEventListener("submit", (event) => { els.saveToken.addEventListener("click", () => withPendingForm(els.authPanel, saveRemoteToken)); els.view.addEventListener("click", (event) => { + const prDiffNavigation = event.target.closest(".pr-diff-nav-item[data-agent-key][data-pull-request-id]"); + if (prDiffNavigation && !event.altKey && !event.ctrlKey && !event.metaKey && !event.shiftKey) { + event.preventDefault(); + cacheVisiblePullRequestDiffViewer(); + showNav(); + navigate({ + type: "agentPullRequests", + key: prDiffNavigation.dataset.agentKey, + pullRequestId: prDiffNavigation.dataset.pullRequestId, + }); + return; + } showNav(); const prDiffLine = event.target.closest("[data-select-pr-diff-line]"); if (prDiffLine) { @@ -15112,11 +15194,16 @@ window.addEventListener("hashchange", () => { closeUnreadPanel(); state.lastScrollY = window.scrollY; showNav(); + resetPullRequestDiffRender(); render(); ensureRouteData({ forceConversation: true, forceProject: true }).then(render).catch((error) => setConnection(error.message)); }); window.addEventListener("scroll", onScroll, { passive: true }); +window.addEventListener("wheel", notePullRequestDiffScrollIntent, { capture: true, passive: true }); +window.addEventListener("touchmove", notePullRequestDiffScrollIntent, { capture: true, passive: true }); +window.addEventListener("pointerdown", notePullRequestDiffScrollIntent, { capture: true, passive: true }); +window.addEventListener("keydown", notePullRequestDiffScrollIntent, true); window.addEventListener("focusin", () => { showNav(); diff --git a/test/managed_agent_test.rb b/test/managed_agent_test.rb index 6a169f1..13c7927 100644 --- a/test/managed_agent_test.rb +++ b/test/managed_agent_test.rb @@ -15,6 +15,7 @@ def run! assert_new_agents_use_unique_log_stems assert_lifetime_run_count_survives_retained_window assert_completed_status_overrides_live_pid + assert_structured_result_status_overrides_transport_exit assert_start_finalizes_unpolled_previous_run assert_cli_status_finalizes_unpolled_dead_pid assert_start_reconciles_session_after_restart @@ -100,6 +101,43 @@ def assert_lifetime_run_count_survives_retained_window end end + def assert_structured_result_status_overrides_transport_exit + run = HQ::ManagedAgent::AgentRun.new( + started_at: Time.utc(2026, 8, 10, 1), + finished_at: Time.utc(2026, 8, 10, 1, 1), + exit_code: 1, + status: "partial" + ) + agent = HQ::ManagedAgent.new( + key: "structured-status-agent", + name: "Structured status", + project_key: "demo", + template_key: "custom", + workspace: Dir.tmpdir, + prompt: "Prompt", + last_exit_code: 1, + runs: [run], + structured_result: { "status" => "partial", "summary" => "Work is ready; CI is flaky." } + ) + + assert(agent.status == "partial", + "expected a validated structured result to prevent a transport exit from rendering as failed") + + agent.structured_result = { "status" => "success", "summary" => "Completed." } + assert(agent.status == "succeeded", + "expected a successful structured result to override a nonzero transport exit") + + agent.structured_result = { "status" => "failed", "summary" => "Validation failed." } + agent.instance_variable_set(:@last_exit_code, 0) + assert(agent.status == "failed", + "expected an explicitly failed structured result to override a zero transport exit") + + agent.structured_result = { "status" => "success", "summary" => "Stopped after completion." } + agent.instance_variable_set(:@last_exit_code, 143) + assert(agent.status == "stopped", + "expected an explicit stop exit to remain a lifecycle state despite a structured result") + end + def assert_completed_run_persists_cost_snapshot Dir.mktmpdir("hq-managed-agent-cost-snapshot-test") do |dir| started_at = Time.utc(2026, 7, 22, 10, 0, 0) From 69cc446a381347afd4497e113b353df31d998241 Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Mon, 10 Aug 2026 08:44:29 +0700 Subject: [PATCH 10/12] Pause workspace polling on focused views --- bin/remote-ui-smoke | 318 ++++++++++++++++++++++++++++++++ docs/PROJECT_STATUS.md | 1 + docs/PULL_REQUEST_DIFFS.md | 2 + lib/hq/remote_ui/assets/app.css | 6 + lib/hq/remote_ui/assets/app.js | 139 ++++++++++---- test/remote_server_test.rb | 12 +- 6 files changed, 443 insertions(+), 35 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index 669ab41..122f3f8 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -3897,6 +3897,324 @@ def write_smoke_script(path) state.renderedViewHtml = ""; render(); }, agentKey); + const focusedCodeHighlightContract = await page.evaluate((key) => { + const originalHash = location.hash; + const originalPrism = window.Prism; + const originalRubyFailure = codeHighlighter.failedLanguages.ruby; + const attachment = { + id: "smoke-code-highlight", + agent_key: key, + type: "file", + title: "smoke.rb", + path: "/tmp/tycho-smoke/smoke.rb", + format: "text", + mime_type: "text/x-ruby", + content: `puts :smoke # ${"x".repeat(500)}`, + }; + state.attachmentDetails[attachment.id] = attachment; + const agent = findAgent(key); + agent.attachments = [...(agent.attachments || []).filter((item) => item.id !== attachment.id), attachment]; + window.Prism = null; + codeHighlighter.failedLanguages.ruby = true; + history.replaceState(null, "", routeHash({ type: "agentAttachment", key, attachmentId: attachment.id })); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + render(); + clearTimeout(state.timer); + state.timer = null; + const workspaceBefore = document.querySelector("[data-agent-workspace]"); + const viewerBefore = document.querySelector(".attachment-code-viewer"); + viewerBefore.scrollLeft = 80; + const codeBefore = viewerBefore.querySelector("code")?.textContent; + window.Prism = { + languages: { ruby: {} }, + highlight: (source) => `${escapeHtml(source)}`, + }; + delete codeHighlighter.failedLanguages.ruby; + renderCodeRoute("ruby"); + const result = { + codeBefore, + highlighted: Boolean(document.querySelector(".attachment-code-viewer .smoke-token")), + scrollLeft: document.querySelector(".attachment-code-viewer")?.scrollLeft, + workspacePreserved: document.querySelector("[data-agent-workspace]") === workspaceBefore, + }; + if (originalPrism === undefined) delete window.Prism; + else window.Prism = originalPrism; + if (originalRubyFailure === undefined) delete codeHighlighter.failedLanguages.ruby; + else codeHighlighter.failedLanguages.ruby = originalRubyFailure; + history.replaceState(null, "", originalHash); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + render(); + schedule(); + return result; + }, agentKey); + if (!focusedCodeHighlightContract.codeBefore.startsWith("puts :smoke") || + !focusedCodeHighlightContract.highlighted || + focusedCodeHighlightContract.scrollLeft !== 80 || + !focusedCodeHighlightContract.workspacePreserved) { + throw new Error(`Focused code highlighting replaced the workspace or failed to upgrade: ${JSON.stringify(focusedCodeHighlightContract)}`); + } + const focusedMarkdownUpgradeContract = await page.evaluate(() => { + const originalMarked = window.marked; + const originalPurify = window.DOMPurify; + const originalFailed = markdownParser.failed; + const workspace = document.querySelector("[data-agent-workspace]"); + const fixture = document.createElement("div"); + fixture.dataset.smokeMarkdownFixture = "true"; + workspace.appendChild(fixture); + window.marked = null; + window.DOMPurify = null; + markdownParser.failed = false; + fixture.innerHTML = renderMarkdown("**focused markdown**", { menuScope: "smoke-focused-markdown" }); + const storedBeforeUpgrade = state.markdownFallbacks.size; + window.marked = { parse: () => "

focused markdown

" }; + window.DOMPurify = { sanitize: (html) => html }; + renderMarkdownRoute(); + const upgraded = Boolean(fixture.querySelector("strong")); + const workspacePreserved = document.querySelector("[data-agent-workspace]") === workspace; + + window.marked = null; + window.DOMPurify = null; + markdownParser.failed = true; + state.markdownFallbacks.clear(); + const failedHtml = renderMarkdown("**plain fallback**", { menuScope: "smoke-failed-markdown" }); + const failedFallbackStored = state.markdownFallbacks.size; + window.marked = originalMarked; + window.DOMPurify = originalPurify; + markdownParser.failed = originalFailed; + fixture.remove(); + return { storedBeforeUpgrade, upgraded, workspacePreserved, failedHtml, failedFallbackStored }; + }); + if (focusedMarkdownUpgradeContract.storedBeforeUpgrade < 1 || + !focusedMarkdownUpgradeContract.upgraded || + !focusedMarkdownUpgradeContract.workspacePreserved || + focusedMarkdownUpgradeContract.failedHtml.includes("data-markdown-fallback-key") || + focusedMarkdownUpgradeContract.failedFallbackStored !== 0) { + throw new Error(`Focused Markdown fallback upgrade or failure cleanup regressed: ${JSON.stringify(focusedMarkdownUpgradeContract)}`); + } + const focusedRoutePollingContract = await page.evaluate(async (key) => { + const originalHash = location.hash; + const originalFetch = window.fetch; + const originalRender = render; + const hiddenDescriptor = Object.getOwnPropertyDescriptor(document, "hidden"); + const savedFullScreenComposerKeys = state.fullScreenComposerKeys; + const savedFullScreenInquiryKeys = state.fullScreenInquiryKeys; + const savedPollDeferredUntil = state.pollDeferredUntil; + const routes = [ + { route: { type: "agentPullRequests", key }, workspacePaused: true }, + { route: { type: "agentAttachment", key, attachmentId: "smoke-attachment-menu" }, workspacePaused: true }, + { route: { type: "agentSummary", key }, workspacePaused: true }, + { route: { type: "projectDiff", key: "web", scope: "worktree", backTo: { type: "agent", key } }, workspacePaused: true }, + { route: { type: "agent", key }, workspacePaused: false }, + ]; + let catalogReads = 0; + let conversationReads = 0; + let renderCalls = 0; + Object.defineProperty(document, "hidden", { configurable: true, get: () => false }); + state.fullScreenComposerKeys = new Set(); + state.fullScreenInquiryKeys = new Set(); + state.pollDeferredUntil = 0; + window.fetch = async (input, init) => { + const requestUrl = new URL(typeof input === "string" ? input : input.url, location.href); + if (requestUrl.pathname === "/servers/resources") catalogReads += 1; + if (requestUrl.pathname.endsWith("/conversation")) conversationReads += 1; + return originalFetch(input, init); + }; + render = (...args) => { + renderCalls += 1; + return originalRender(...args); + }; + try { + const results = []; + for (const { route, workspacePaused } of routes) { + clearTimeout(state.timer); + state.timer = null; + catalogReads = 0; + conversationReads = 0; + renderCalls = 0; + history.replaceState(null, "", routeHash(route)); + await refresh({ force: true, forceConversation: true, preserveFocusedWorkspace: true }); + results.push({ type: route.type, workspacePaused, catalogReads, conversationReads, renderCalls }); + } + return results; + } finally { + window.fetch = originalFetch; + render = originalRender; + if (hiddenDescriptor) Object.defineProperty(document, "hidden", hiddenDescriptor); + else delete document.hidden; + state.fullScreenComposerKeys = savedFullScreenComposerKeys; + state.fullScreenInquiryKeys = savedFullScreenInquiryKeys; + state.pollDeferredUntil = savedPollDeferredUntil; + clearTimeout(state.timer); + state.timer = null; + history.replaceState(null, "", originalHash); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + render(); + schedule(); + } + }, agentKey); + if (focusedRoutePollingContract.some((route) => ( + route.catalogReads < 1 || + (route.workspacePaused && (route.conversationReads !== 0 || route.renderCalls !== 0)) || + (!route.workspacePaused && (route.conversationReads < 1 || route.renderCalls < 1)) + ))) { + throw new Error(`Focused non-conversation routes performed workspace polling: ${JSON.stringify(focusedRoutePollingContract)}`); + } + const focusedStatusSyncContract = await page.evaluate(async (key) => { + const originalHash = location.hash; + const originalLoadResourceCatalog = loadResourceCatalog; + const originalRender = render; + const agent = findAgent(key); + agent.running = true; + agent.status = "running"; + agent.unread = false; + history.replaceState(null, "", routeHash({ type: "agentSummary", key })); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + originalRender(); + clearTimeout(state.timer); + state.timer = null; + const workspaceBefore = document.querySelector("[data-agent-workspace]"); + const steadyActionBefore = document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"] [data-agent-action="stop"]`); + steadyActionBefore?.focus({ preventScroll: true }); + syncFocusedWorkspaceCatalog(parseRoute()); + const steadyActionAfter = document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"] [data-agent-action="stop"]`); + const before = { + status: document.querySelector(`[data-agent-live-status="${CSS.escape(key)}"]`)?.textContent.trim(), + action: document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"] [data-agent-action="stop"]`)?.textContent.trim(), + steadyActionPreserved: steadyActionAfter === steadyActionBefore, + steadyActionFocused: document.activeElement === steadyActionBefore, + }; + let renderCalls = 0; + let conversationReads = 0; + render = (...args) => { + renderCalls += 1; + return originalRender(...args); + }; + loadResourceCatalog = async () => { + const data = await originalLoadResourceCatalog(); + for (const server of data.servers || []) { + for (const item of server.agents || []) { + if (normalizeAgentResource(item, server)?.key !== key) continue; + item.running = false; + item.status = "succeeded"; + item.unread = false; + item.awaiting_input = false; + item.blocked = false; + item.last_result = "success"; + } + } + return data; + }; + const originalFetch = window.fetch; + window.fetch = async (input, init) => { + const requestUrl = new URL(typeof input === "string" ? input : input.url, location.href); + if (requestUrl.pathname.endsWith("/conversation")) conversationReads += 1; + return originalFetch(input, init); + }; + try { + await refresh({ preserveFocusedWorkspace: true }); + return { + before, + after: { + status: document.querySelector(`[data-agent-live-status="${CSS.escape(key)}"]`)?.textContent.trim(), + sendPrompt: document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"] button[type="submit"]`)?.textContent.trim(), + sendPromptFocused: document.activeElement === document.querySelector(`#composer[data-agent-key="${CSS.escape(key)}"] button[type="submit"]`), + }, + conversationReads, + renderCalls, + workspacePreserved: document.querySelector("[data-agent-workspace]") === workspaceBefore, + }; + } finally { + window.fetch = originalFetch; + loadResourceCatalog = originalLoadResourceCatalog; + render = originalRender; + clearTimeout(state.timer); + state.timer = null; + history.replaceState(null, "", originalHash); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + originalRender(); + schedule(); + } + }, agentKey); + if (focusedStatusSyncContract.before.status !== "Running" || + focusedStatusSyncContract.before.action !== "Stop agent" || + !focusedStatusSyncContract.before.steadyActionPreserved || + !focusedStatusSyncContract.before.steadyActionFocused || + focusedStatusSyncContract.after.status !== "Succeeded" || + focusedStatusSyncContract.after.sendPrompt !== "Send prompt" || + !focusedStatusSyncContract.after.sendPromptFocused || + focusedStatusSyncContract.conversationReads !== 0 || + !focusedStatusSyncContract.workspacePreserved) { + throw new Error(`Focused workspace status controls did not reconcile in place: ${JSON.stringify(focusedStatusSyncContract)}`); + } + const navigationDuringPollContract = await page.evaluate(async (key) => { + const originalHash = location.hash; + const originalLoadResourceCatalog = loadResourceCatalog; + const originalFetch = window.fetch; + const originalRender = render; + let releaseCatalog; + let catalogEntered; + const catalogStarted = new Promise((resolve) => { catalogEntered = resolve; }); + const catalogGate = new Promise((resolve) => { releaseCatalog = resolve; }); + let conversationReads = 0; + let renderCalls = 0; + history.replaceState(null, "", routeHash({ type: "agent", key })); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + originalRender(); + loadResourceCatalog = async () => { + catalogEntered(); + await catalogGate; + return originalLoadResourceCatalog(); + }; + window.fetch = async (input, init) => { + const requestUrl = new URL(typeof input === "string" ? input : input.url, location.href); + if (requestUrl.pathname.endsWith("/conversation")) conversationReads += 1; + return originalFetch(input, init); + }; + render = (...args) => { + renderCalls += 1; + return originalRender(...args); + }; + try { + const pendingRefresh = refresh({ force: true, forceConversation: true }); + await catalogStarted; + history.replaceState(null, "", routeHash({ type: "agentSummary", key })); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + originalRender(); + releaseCatalog(); + await pendingRefresh; + return { + route: parseRoute().type, + summaryPresent: Boolean(document.querySelector("[data-agent-summary-page]")), + conversationReads, + renderCalls, + }; + } finally { + releaseCatalog(); + window.fetch = originalFetch; + loadResourceCatalog = originalLoadResourceCatalog; + render = originalRender; + clearTimeout(state.timer); + state.timer = null; + history.replaceState(null, "", originalHash); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + originalRender(); + schedule(); + } + }, agentKey); + if (navigationDuringPollContract.route !== "agentSummary" || + !navigationDuringPollContract.summaryPresent || + navigationDuringPollContract.conversationReads !== 0 || + navigationDuringPollContract.renderCalls !== 0) { + throw new Error(`An older conversation poll replaced focused navigation: ${JSON.stringify(navigationDuringPollContract)}`); + } const attachmentPollRenderContract = await page.evaluate(async () => { clearTimeout(state.timer); state.timer = null; diff --git a/docs/PROJECT_STATUS.md b/docs/PROJECT_STATUS.md index f962a9f..0cc0d20 100644 --- a/docs/PROJECT_STATUS.md +++ b/docs/PROJECT_STATUS.md @@ -353,6 +353,7 @@ verify the bottle. - [x] Dedicated mobile structured inquiry submission UI - [x] Full-screen inquiry editor with trailing unstructured Leave feedback field - [x] Poll-safe inline and full-screen Conversation/inquiry forms that remain attached and focused during shell refreshes +- [x] Focused Summary, Attachment, PR Diff, and agent-backed project Diff routes poll only the lightweight resource catalog, preserve their workspace DOM, and reconcile status/lifecycle controls in place; normal route polling resumes on Conversation and top-level live views - [x] Attachment detail context menus with exclusive Balanced/Widen/Full layouts, content/path copy, and forced cache refresh - [x] Responsive desktop/mobile shell with persistent, header-aligned in-shell desktop navigation across top-level and detail routes, consistently named New agent action, shared control sizing, and fixed-region safe-area handling - [x] Sticky Settings section navigator over one continuous page and copyable native session ID in Conversation Settings diff --git a/docs/PULL_REQUEST_DIFFS.md b/docs/PULL_REQUEST_DIFFS.md index 81bd8eb..c82bf3a 100644 --- a/docs/PULL_REQUEST_DIFFS.md +++ b/docs/PULL_REQUEST_DIFFS.md @@ -12,6 +12,8 @@ Agent-scoped PR discovery persists canonical references and compact metadata in The Remote UI preloads up to six saved diff snapshots for the open agent, cancels stale queued preloads when that route changes, retains at most twelve snapshot payloads, and keeps up to six visited diff viewers and scroll positions in bounded LRU caches. Foreground navigation has priority and one reserved request slot, so a click cannot wait behind background preloads; an explicit refresh that meets an in-flight preload follows it with the required refresh request. Single and bulk refresh results use the same bounded payload cache and protect the selected PR. Route changes paint the PR shell and loading state first, then attach even an already-cached diff on a later animation frame, keeping diff layout out of the navigation frame. Diff lines render in 100-line containment chunks so off-screen code does not force full-page layout. The persistent snapshot store remains authoritative, while the server reuses an immutable parsed document until the snapshot file's inode, size, or nanosecond mtime changes. Switching among visited PRs therefore avoids both JSON reparsing and rebuilding thousands of diff-line DOM controls; first visits use the preloaded snapshot and bounded layout. +Automatic Remote UI polling does not fetch or rebuild the conversation behind a focused PR Diff. It still reconciles lightweight agent status and lifecycle controls from the resource catalog; the cross-route policy is recorded in `docs/PROJECT_STATUS.md`. + ## Workflow ```mermaid diff --git a/lib/hq/remote_ui/assets/app.css b/lib/hq/remote_ui/assets/app.css index eef05a8..147ba99 100644 --- a/lib/hq/remote_ui/assets/app.css +++ b/lib/hq/remote_ui/assets/app.css @@ -5997,6 +5997,12 @@ body:has(.inquiry-form-full-screen) { gap: 8px; } +.agent-live-actions { + display: inline-flex; + align-items: center; + gap: 8px; +} + .agent-running-indicator { display: inline-grid; place-items: center; diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index e9fb53e..c4b4e9d 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -63,6 +63,7 @@ const MARKDOWN_SCRIPT_URLS = { dompurify: "https://cdn.jsdelivr.net/npm/dompurify@3.2.7/dist/purify.min.js", marked: "https://cdn.jsdelivr.net/npm/marked@18.0.3/lib/marked.umd.js", }; +const MARKDOWN_FALLBACK_LIMIT = 200; const MERMAID_SCRIPT_URL = "https://cdn.jsdelivr.net/npm/mermaid@11.15.0/dist/mermaid.min.js"; const CODE_HIGHLIGHTER_SCRIPT_URLS = { prism: "https://cdn.jsdelivr.net/npm/prismjs@1.30.0/prism.min.js", @@ -670,6 +671,8 @@ const state = { pullRequestDiffsInFlight: 0, openBlockMenu: null, openMarkdownCodeMenu: null, + markdownFallbacks: new Map(), + markdownFallbackSequence: 0, agentSettingsOpen: false, headerMoreOpen: false, headerMoreBadge: "", @@ -1683,10 +1686,58 @@ function deferPollAfterFormInput(event) { } function preserveWorkspaceDuringPoll(options = {}) { - if (options.force || options.forceAttachment) return false; + if (options.forceAttachment) return false; + if (options.force && !options.preserveFocusedWorkspace) return false; if (els.view.querySelector("#inquiry-form")) return true; - return ["agentAttachment", "attachment"].includes(parseRoute().type); + return focusedWorkspacePreservedDuringPoll(); +} + +function focusedWorkspacePreservedDuringPoll(route = parseRoute()) { + return ["agentAttachment", "agentPullRequests", "agentSummary", "attachment", "projectDiff"].includes(route.type); +} + +function focusedWorkspaceAgent(route = parseRoute()) { + const workspaceRoute = agentWorkspaceRoute(route); + if (workspaceRoute) return findAgent(workspaceRoute.key); + if (route.type !== "attachment") return null; + + const attachment = state.attachmentDetails[route.id] || attachmentById(route.id); + return attachment?.agent_key ? findAgent(attachment.agent_key) : null; +} + +function syncFocusedWorkspaceCatalog(route = parseRoute()) { + if (!focusedWorkspacePreservedDuringPoll(route)) return; + const agent = focusedWorkspaceAgent(route); + if (!agent) return; + + setHeaderMore(agentMoreMenuHtml(agent), "Conversation actions", agentHeaderMoreKey(agent.key)); + syncHeaderScheduleRoute(route); + if (state.agentSettingsOpen) setAgentSettings(agent); + + document.querySelectorAll(`[data-agent-live-status="${CSS.escape(agent.key)}"]`).forEach((element) => { + element.innerHTML = statusBadge(statusLabel(agent), statusClass(agent), "chip"); + }); + + const composer = document.querySelector(`#composer[data-agent-key="${CSS.escape(agent.key)}"]`); + if (!composer) return; + const running = agentIsRunning(agent); + composer.dataset.agentRunning = running ? "true" : "false"; + const actions = composer.querySelector("[data-agent-live-actions]"); + const sending = state.pendingComposerKeys.has(agent.key) || state.pendingPullRequestCommentKeys.has(agent.key); + const actionState = agentComposerActionState(agent, sending); + if (actions && actions.dataset.agentLiveActionState !== actionState) { + const restoreActionFocus = actions.contains(document.activeElement); + actions.dataset.agentLiveActionState = actionState; + actions.innerHTML = ` + ${running ? `${iconSvg("loaderPinwheel")}` : ""} + ${agentComposerAction(agent, sending)} + `; + if (restoreActionFocus) actions.querySelector("button")?.focus({ preventScroll: true }); + } + composer.querySelectorAll("[data-prompt-attachment-input], [data-add-prompt-attachment], [data-toggle-speech-mode], [data-toggle-skills]").forEach((control) => { + control.disabled = running; + }); } async function refresh(options = {}) { @@ -1703,6 +1754,7 @@ async function refresh(options = {}) { return; } + const initialRouteHash = location.hash; const preserveWorkspace = preserveWorkspaceDuringPoll(options); const inquiryAgentKey = preserveWorkspace ? els.view.querySelector("#inquiry-form")?.dataset.agentKey @@ -1738,14 +1790,18 @@ async function refresh(options = {}) { state.lastUpdatedAt = new Date(); state.failureCount = 0; els.authPanel.classList.add("hidden"); - if (!preserveWorkspace) await ensureRouteData(options); + const routeChangedDuringRefresh = location.hash !== initialRouteHash; + const preserveCurrentWorkspace = routeChangedDuringRefresh || preserveWorkspaceDuringPoll(options); + if (!preserveCurrentWorkspace) await ensureRouteData(options); setConnection(refreshedText()); const currentRoute = parseRoute(); if (shouldOpenSucceededSummary && currentRoute.type === "agent" && currentRoute.key === routeBeforeRefresh.key) { navigate({ type: "agentSummary", key: currentRoute.key }); return; } - if (!preserveWorkspace) { + if (preserveCurrentWorkspace) { + syncFocusedWorkspaceCatalog(currentRoute); + } else { render({ preserveLiveEditor: true, preservePollContent: !options.force && !options.forceAttachment, @@ -1760,7 +1816,7 @@ async function refresh(options = {}) { } else { setConnection(`Offline: ${error.message}`); } - if (!preserveWorkspace) { + if (!preserveWorkspaceDuringPoll(options) && location.hash === initialRouteHash) { render({ preserveLiveEditor: true, preservePollContent: !options.force && !options.forceAttachment, @@ -5213,6 +5269,7 @@ function renderAgentSummaryView(agent, options = {}) {
Summary${escapeHtml(meta || agent.name || agent.key)}
+
Current session${statusBadge(statusLabel(agent), statusClass(agent), "chip")}
${kv("Outcome", summary.status || statusLabel(agent))} ${kv("Total Runs", agent.run_count || 0)} @@ -5905,8 +5962,10 @@ function renderAgentComposer(agent, skills, options = {}) {
- ${agentIsRunning(agent) ? `${iconSvg("loaderPinwheel")}` : ""} - ${agentComposerAction(agent, sending)} + + ${agentIsRunning(agent) ? `${iconSvg("loaderPinwheel")}` : ""} + ${agentComposerAction(agent, sending)} +
@@ -6200,6 +6259,11 @@ function agentComposerAction(agent, sending = false) { return ``; } +function agentComposerActionState(agent, sending = false) { + if (agentIsRunning(agent)) return "running"; + return sending ? "sending" : "idle"; +} + function renderSpeechModeButton(agent) { const available = speechModeAvailable(); const active = state.speechComposerKey === agent.key && Boolean(state.speechRecognition); @@ -9395,20 +9459,31 @@ function highlightedCodeHtml(text, language) { function renderCodeRoute(language) { const route = parseRoute(); - if (route.type === "agent" || route.type === "agentSummary" || route.type === "agentAttachment") { + if (route.type === "agent") { state.renderedViewHtml = ""; render(); return; } - if (route.type !== "attachment") return; - const attachment = state.attachmentDetails[route.id] || attachmentById(route.id); + if (route.type !== "attachment" && route.type !== "agentAttachment") return; + const attachmentId = route.type === "agentAttachment" ? route.attachmentId : route.id; + const attachment = state.attachmentDetails[attachmentId] || attachmentById(attachmentId); if (!attachment || attachmentKind(attachment) !== "file") return; if (attachmentFormat(attachment) !== "text") return; if (language && codeLanguageForAttachment(attachment) !== language) return; - state.renderedViewHtml = ""; - render(); + const viewer = els.view.querySelector(".attachment-code-viewer"); + if (!viewer) return; + const template = document.createElement("template"); + template.innerHTML = renderCodeAttachment(attachment.content, attachment); + const replacement = template.content.firstElementChild; + if (replacement) { + const scrollTop = viewer.scrollTop; + const scrollLeft = viewer.scrollLeft; + viewer.replaceWith(replacement); + replacement.scrollTop = scrollTop; + replacement.scrollLeft = scrollLeft; + } } function openAttachmentLinkOnce(id, href) { @@ -9421,9 +9496,16 @@ function openAttachmentLinkOnce(id, href) { function renderMarkdown(text, options = {}) { const source = String(text || ""); if (markdownParserReady()) return renderParsedMarkdown(source, options); + if (markdownParser.failed) return renderPlainTextMarkdown(source, options); ensureMarkdownParserLoaded(); - return renderPlainTextMarkdown(source, options); + state.markdownFallbackSequence += 1; + const fallbackKey = `markdown-fallback:${state.markdownFallbackSequence}`; + while (state.markdownFallbacks.size >= MARKDOWN_FALLBACK_LIMIT) { + state.markdownFallbacks.delete(state.markdownFallbacks.keys().next().value); + } + state.markdownFallbacks.set(fallbackKey, { source, options: { ...options } }); + return renderPlainTextMarkdown(source, { ...options, fallbackKey }); } function markdownParserReady() { @@ -9444,6 +9526,7 @@ function ensureMarkdownParserLoaded() { return true; }).catch((error) => { markdownParser.failed = true; + state.markdownFallbacks.clear(); console.warn("Markdown parser load failed", error); return false; }); @@ -9477,23 +9560,16 @@ function loadExternalScript(src) { } function renderMarkdownRoute() { - const route = parseRoute(); - if (route.type === "agent" || route.type === "agentSummary" || route.type === "agentAttachment") { - state.renderedViewHtml = ""; - render(); - return; - } - - if (route.type !== "attachment" && route.type !== "agentAttachment") return; + els.view.querySelectorAll("[data-markdown-fallback-key]").forEach((element) => { + const fallbackKey = element.dataset.markdownFallbackKey; + const fallback = state.markdownFallbacks.get(fallbackKey); + if (!fallback) return; - const attachmentId = route.type === "agentAttachment" ? route.attachmentId : route.id; - const attachment = state.attachmentDetails[attachmentId] || attachmentById(attachmentId); - if (!attachment || attachmentKind(attachment) !== "file") return; - const format = String(attachment.format || attachmentFormat(attachment)).toLowerCase(); - if (format !== "markdown") return; - - state.renderedViewHtml = ""; - render(); + element.outerHTML = renderParsedMarkdown(fallback.source, fallback.options); + state.markdownFallbacks.delete(fallbackKey); + }); + state.markdownFallbacks.clear(); + queueMermaidRendering(); } function renderParsedMarkdown(text, options = {}) { @@ -9621,7 +9697,8 @@ function markMermaidError(node, source) { function renderPlainTextMarkdown(text, options = {}) { const className = String(options.fallbackClassName || "attachment-text-viewer").trim() || "attachment-text-viewer"; const emptyText = Object.prototype.hasOwnProperty.call(options, "emptyText") ? options.emptyText : "No content"; - return `
${escapeHtml(text || emptyText)}
`; + const fallbackKey = options.fallbackKey ? ` data-markdown-fallback-key="${escapeAttr(options.fallbackKey)}"` : ""; + return `
${escapeHtml(text || emptyText)}
`; } function markdownViewerClassName(options = {}) { @@ -15230,7 +15307,7 @@ document.addEventListener("visibilitychange", () => { state.failureCount = 0; state.lastScrollY = window.scrollY; showNav(); - refresh({ force: true }); + refresh({ force: true, preserveFocusedWorkspace: true }); } else { flushComposerDraftSaves(); } diff --git a/test/remote_server_test.rb b/test/remote_server_test.rb index f6b562f..f19833c 100644 --- a/test/remote_server_test.rb +++ b/test/remote_server_test.rb @@ -5121,12 +5121,16 @@ def assert_remote_ui_routes_load_without_auth js[:body].include?("delete state.agentDetails[agent.key]"), "expected catalog revision changes to invalidate stale agent attachment details") assert(js[:body].include?("function preserveWorkspaceDuringPoll") && - js[:body].include?("if (options.force || options.forceAttachment) return false") && + js[:body].include?("if (options.forceAttachment) return false") && + js[:body].include?("if (options.force && !options.preserveFocusedWorkspace) return false") && + js[:body].include?("function focusedWorkspacePreservedDuringPoll") && + js[:body].include?("function syncFocusedWorkspaceCatalog") && js[:body].include?('els.view.querySelector("#inquiry-form")') && js[:body].include?("if (preservedInquiryAgent) state.agentDetails[inquiryAgentKey] = preservedInquiryAgent") && - js[:body].include?("if (!preserveWorkspace) await ensureRouteData(options)") && - js[:body].scan("if (!preserveWorkspace)").length == 3, - "expected automatic attachment and inquiry polling to update state without rendering the workspace") + js[:body].include?("const preserveCurrentWorkspace = routeChangedDuringRefresh || preserveWorkspaceDuringPoll(options)") && + js[:body].include?("if (!preserveCurrentWorkspace) await ensureRouteData(options)") && + js[:body].include?("syncFocusedWorkspaceCatalog(currentRoute)"), + "expected focused workspace polling to reconcile catalog state without fetching or rendering route data") assert(js[:body].include?("state.preservePollContentDuringRender") && js[:body].include?("replaceViewContent(html)"), "expected explicit attachment refreshes to replace preserved attachment content") From 78dd0ecd1c4cdc6735affbccd6cb877c45be4308 Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Mon, 10 Aug 2026 10:39:41 +0700 Subject: [PATCH 11/12] Show paused polling state on focused views --- bin/remote-ui-smoke | 54 +++++++++++++++++++++++++++++---- docs/PROJECT_STATUS.md | 2 +- lib/hq/remote_ui/assets/app.css | 6 ++++ lib/hq/remote_ui/assets/app.js | 40 +++++++++++++++++++++--- test/remote_server_test.rb | 2 ++ 5 files changed, 93 insertions(+), 11 deletions(-) diff --git a/bin/remote-ui-smoke b/bin/remote-ui-smoke index 122f3f8..2bfa387 100755 --- a/bin/remote-ui-smoke +++ b/bin/remote-ui-smoke @@ -4002,10 +4002,10 @@ def write_smoke_script(path) const savedFullScreenInquiryKeys = state.fullScreenInquiryKeys; const savedPollDeferredUntil = state.pollDeferredUntil; const routes = [ - { route: { type: "agentPullRequests", key }, workspacePaused: true }, - { route: { type: "agentAttachment", key, attachmentId: "smoke-attachment-menu" }, workspacePaused: true }, - { route: { type: "agentSummary", key }, workspacePaused: true }, - { route: { type: "projectDiff", key: "web", scope: "worktree", backTo: { type: "agent", key } }, workspacePaused: true }, + { route: { type: "agentPullRequests", key }, workspacePaused: true, subtitle: "Conversation poll paused / PR details" }, + { route: { type: "agentAttachment", key, attachmentId: "smoke-attachment-menu" }, workspacePaused: true, subtitle: "Conversation poll paused / attachment" }, + { route: { type: "agentSummary", key }, workspacePaused: true, subtitle: "Conversation poll paused / summary" }, + { route: { type: "projectDiff", key: "web", scope: "worktree", backTo: { type: "agent", key } }, workspacePaused: true, subtitle: "Conversation poll paused / project diff" }, { route: { type: "agent", key }, workspacePaused: false }, ]; let catalogReads = 0; @@ -4027,7 +4027,7 @@ def write_smoke_script(path) }; try { const results = []; - for (const { route, workspacePaused } of routes) { + for (const { route, workspacePaused, subtitle } of routes) { clearTimeout(state.timer); state.timer = null; catalogReads = 0; @@ -4035,7 +4035,7 @@ def write_smoke_script(path) renderCalls = 0; history.replaceState(null, "", routeHash(route)); await refresh({ force: true, forceConversation: true, preserveFocusedWorkspace: true }); - results.push({ type: route.type, workspacePaused, catalogReads, conversationReads, renderCalls }); + results.push({ type: route.type, workspacePaused, expectedSubtitle: subtitle, subtitle: els.subtitle.textContent.trim(), catalogReads, conversationReads, renderCalls }); } return results; } finally { @@ -4057,11 +4057,53 @@ def write_smoke_script(path) }, agentKey); if (focusedRoutePollingContract.some((route) => ( route.catalogReads < 1 || + (route.workspacePaused && route.subtitle !== route.expectedSubtitle) || (route.workspacePaused && (route.conversationReads !== 0 || route.renderCalls !== 0)) || (!route.workspacePaused && (route.conversationReads < 1 || route.renderCalls < 1)) ))) { throw new Error(`Focused non-conversation routes performed workspace polling: ${JSON.stringify(focusedRoutePollingContract)}`); } + const mobilePollingSubtitleHash = await page.evaluate(() => location.hash); + await page.setViewportSize({ width: 390, height: 844 }); + const mobilePollingSubtitleContract = await page.evaluate((key) => { + history.replaceState(null, "", routeHash({ type: "agentPullRequests", key })); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + render(); + const subtitle = document.querySelector("#screen-subtitle .subtitle-status-text"); + const styles = getComputedStyle(subtitle); + const headerRect = els.header.getBoundingClientRect(); + const firstContentRect = els.view.firstElementChild?.getBoundingClientRect(); + const result = { + text: subtitle?.textContent.trim(), + clientWidth: subtitle?.clientWidth || 0, + scrollWidth: subtitle?.scrollWidth || 0, + clientHeight: subtitle?.clientHeight || 0, + scrollHeight: subtitle?.scrollHeight || 0, + whiteSpace: styles.whiteSpace, + overflow: styles.overflow, + headerHeight: headerRect.height, + headerBottom: headerRect.bottom, + firstContentTop: firstContentRect?.top || 0, + detailHeaderHeight: parseFloat(getComputedStyle(els.view).getPropertyValue("--detail-header-height")) || 0, + }; + return result; + }, agentKey); + if (!mobilePollingSubtitleContract.text.startsWith("Conversation poll paused") || + mobilePollingSubtitleContract.whiteSpace === "nowrap" || + mobilePollingSubtitleContract.scrollWidth > mobilePollingSubtitleContract.clientWidth + 1 || + mobilePollingSubtitleContract.scrollHeight > mobilePollingSubtitleContract.clientHeight + 1 || + Math.abs(mobilePollingSubtitleContract.detailHeaderHeight - Math.ceil(mobilePollingSubtitleContract.headerHeight)) > 1 || + mobilePollingSubtitleContract.firstContentTop + 1 < mobilePollingSubtitleContract.headerBottom) { + throw new Error(`Mobile header clips the paused polling state: ${JSON.stringify(mobilePollingSubtitleContract)}`); + } + await page.setViewportSize({ width: 1280, height: 800 }); + await page.evaluate((hash) => { + history.replaceState(null, "", hash); + state.renderedRouteKey = null; + state.renderedViewHtml = ""; + render(); + }, mobilePollingSubtitleHash); const focusedStatusSyncContract = await page.evaluate(async (key) => { const originalHash = location.hash; const originalLoadResourceCatalog = loadResourceCatalog; diff --git a/docs/PROJECT_STATUS.md b/docs/PROJECT_STATUS.md index 0cc0d20..4a183e3 100644 --- a/docs/PROJECT_STATUS.md +++ b/docs/PROJECT_STATUS.md @@ -353,7 +353,7 @@ verify the bottle. - [x] Dedicated mobile structured inquiry submission UI - [x] Full-screen inquiry editor with trailing unstructured Leave feedback field - [x] Poll-safe inline and full-screen Conversation/inquiry forms that remain attached and focused during shell refreshes -- [x] Focused Summary, Attachment, PR Diff, and agent-backed project Diff routes poll only the lightweight resource catalog, preserve their workspace DOM, and reconcile status/lifecycle controls in place; normal route polling resumes on Conversation and top-level live views +- [x] Focused Summary, Attachment, PR Diff, and agent-backed project Diff routes show that conversation polling is paused, poll only the lightweight resource catalog, preserve their workspace DOM, and reconcile status/lifecycle controls in place; normal route polling resumes on Conversation and top-level live views - [x] Attachment detail context menus with exclusive Balanced/Widen/Full layouts, content/path copy, and forced cache refresh - [x] Responsive desktop/mobile shell with persistent, header-aligned in-shell desktop navigation across top-level and detail routes, consistently named New agent action, shared control sizing, and fixed-region safe-area handling - [x] Sticky Settings section navigator over one continuous page and copyable native session ID in Conversation Settings diff --git a/lib/hq/remote_ui/assets/app.css b/lib/hq/remote_ui/assets/app.css index 147ba99..e75e10f 100644 --- a/lib/hq/remote_ui/assets/app.css +++ b/lib/hq/remote_ui/assets/app.css @@ -7162,6 +7162,12 @@ body:has(.inquiry-form-full-screen) { white-space: nowrap; } + .subtitle-polling-paused .subtitle-status-text { + overflow: visible; + text-overflow: clip; + white-space: normal; + } + .content { padding-bottom: calc(var(--mobile-nav-height) + 24px + env(safe-area-inset-bottom, 0px)); } diff --git a/lib/hq/remote_ui/assets/app.js b/lib/hq/remote_ui/assets/app.js index c4b4e9d..fd8c7b8 100644 --- a/lib/hq/remote_ui/assets/app.js +++ b/lib/hq/remote_ui/assets/app.js @@ -1697,6 +1697,27 @@ function focusedWorkspacePreservedDuringPoll(route = parseRoute()) { return ["agentAttachment", "agentPullRequests", "agentSummary", "attachment", "projectDiff"].includes(route.type); } +function focusedWorkspacePollingSubtitle(route = parseRoute()) { + const detail = { + agentAttachment: "attachment", + agentPullRequests: "PR details", + agentSummary: "summary", + attachment: "attachment", + projectDiff: "project diff", + }[route.type]; + return detail ? `Conversation poll paused / ${detail}` : ""; +} + +function syncFocusedWorkspacePollingSubtitle(route = parseRoute()) { + const subtitle = focusedWorkspacePollingSubtitle(route); + if (!subtitle) return false; + + setHeaderSubtitleIcon("squareSlash"); + setHeaderSubtitle(subtitle); + syncDetailHeaderLayout(); + return true; +} + function focusedWorkspaceAgent(route = parseRoute()) { const workspaceRoute = agentWorkspaceRoute(route); if (workspaceRoute) return findAgent(workspaceRoute.key); @@ -1761,7 +1782,8 @@ async function refresh(options = {}) { : ""; const preservedInquiryAgent = inquiryAgentKey ? state.agentDetails[inquiryAgentKey] : null; try { - setConnection("Refreshing"); + const preserveInitialSubtitle = preserveWorkspace && syncFocusedWorkspacePollingSubtitle(parseRoute()); + setConnection("Refreshing", { preserveHeaderSubtitle: preserveInitialSubtitle }); const catalogData = await loadResourceCatalog(); const previousAgents = state.agents; applyResourceCatalog(catalogData); @@ -1793,8 +1815,12 @@ async function refresh(options = {}) { const routeChangedDuringRefresh = location.hash !== initialRouteHash; const preserveCurrentWorkspace = routeChangedDuringRefresh || preserveWorkspaceDuringPoll(options); if (!preserveCurrentWorkspace) await ensureRouteData(options); - setConnection(refreshedText()); const currentRoute = parseRoute(); + const focusedPollingSubtitle = preserveCurrentWorkspace && focusedWorkspacePollingSubtitle(currentRoute); + setConnection(refreshedText(), { + preserveHeaderSubtitle: routeChangedDuringRefresh || Boolean(focusedPollingSubtitle), + }); + if (focusedPollingSubtitle) syncFocusedWorkspacePollingSubtitle(currentRoute); if (shouldOpenSucceededSummary && currentRoute.type === "agent" && currentRoute.key === routeBeforeRefresh.key) { navigate({ type: "agentSummary", key: currentRoute.key }); return; @@ -2552,6 +2578,7 @@ function renderCurrentView() { } else { renderNow(); } + syncFocusedWorkspacePollingSubtitle(route); } function replaceView(html) { @@ -10006,7 +10033,11 @@ function setHeaderSubtitle(text) { function renderHeaderSubtitle() { const value = state.headerSubtitle; const icon = state.refreshing ? iconSvg("hourglass") : iconSvg(state.headerSubtitleIcon); - const className = state.refreshing ? "subtitle-status subtitle-refresh" : "subtitle-status"; + const className = [ + "subtitle-status", + state.refreshing ? "subtitle-refresh" : "", + value.startsWith("Conversation poll paused") ? "subtitle-polling-paused" : "", + ].filter(Boolean).join(" "); els.subtitle.innerHTML = `${escapeHtml(value)}`; } @@ -10964,9 +10995,10 @@ function responseStyleSourceLabel(source) { }[String(source || "").toLowerCase()] || "Disabled"; } -function setConnection(text) { +function setConnection(text, options = {}) { state.connection = text; state.refreshing = text === "Refreshing"; + if (options.preserveHeaderSubtitle) return; if (state.refreshing) { renderHeaderSubtitle(); } else { diff --git a/test/remote_server_test.rb b/test/remote_server_test.rb index f19833c..6e11498 100644 --- a/test/remote_server_test.rb +++ b/test/remote_server_test.rb @@ -5124,6 +5124,8 @@ def assert_remote_ui_routes_load_without_auth js[:body].include?("if (options.forceAttachment) return false") && js[:body].include?("if (options.force && !options.preserveFocusedWorkspace) return false") && js[:body].include?("function focusedWorkspacePreservedDuringPoll") && + js[:body].include?("function focusedWorkspacePollingSubtitle") && + js[:body].include?("Conversation poll paused") && js[:body].include?("function syncFocusedWorkspaceCatalog") && js[:body].include?('els.view.querySelector("#inquiry-form")') && js[:body].include?("if (preservedInquiryAgent) state.agentDetails[inquiryAgentKey] = preservedInquiryAgent") && From d5100aab66829b4f797319bc4d038c3899935e5e Mon Sep 17 00:00:00 2001 From: firewalker06 Date: Tue, 11 Aug 2026 01:58:04 +0700 Subject: [PATCH 12/12] Clear stale Bundler setup from agent runners --- lib/hq/domain/managed_agent.rb | 1 + test/managed_agent_test.rb | 16 ++++++++++++++++ 2 files changed, 17 insertions(+) diff --git a/lib/hq/domain/managed_agent.rb b/lib/hq/domain/managed_agent.rb index 8da7481..e3b4cfb 100644 --- a/lib/hq/domain/managed_agent.rb +++ b/lib/hq/domain/managed_agent.rb @@ -1138,6 +1138,7 @@ def external_process_environment(environment) { "BUNDLE_BIN_PATH" => nil, "BUNDLE_GEMFILE" => nil, + "BUNDLER_SETUP" => nil, "BUNDLER_VERSION" => nil, "GEM_HOME" => nil, "GEM_PATH" => nil, diff --git a/test/managed_agent_test.rb b/test/managed_agent_test.rb index 13c7927..9c36ee3 100644 --- a/test/managed_agent_test.rb +++ b/test/managed_agent_test.rb @@ -1542,10 +1542,26 @@ def assert_external_process_environment_removes_ruby_loader_state env = agent.send(:external_process_environment, "BUNDLE_GEMFILE" => "/custom/Gemfile", "CUSTOM" => "1") assert(env["BUNDLE_BIN_PATH"].nil?, "expected Bundler bin path to be cleared for harnesses") + assert(env.key?("BUNDLER_SETUP") && env["BUNDLER_SETUP"].nil?, + "expected Bundler setup hook to be cleared for harnesses") assert(env["RUBYOPT"].nil?, "expected Ruby loader options to be cleared for harnesses") assert(env["GEM_HOME"].nil?, "expected Ruby gem home to be cleared for harnesses") assert(env["BUNDLE_GEMFILE"] == "/custom/Gemfile", "expected explicit harness env to remain authoritative") assert(env["CUSTOM"] == "1", "expected explicit harness env to be preserved") + + runner_output = IO.popen( + agent.send(:external_process_environment, {}), + [ + RbConfig.ruby, + "-I", File.expand_path("../lib", __dir__), + "-r", "hq/domain/agent_correction_runner", + "-e", 'STDOUT.write("runner-loaded")' + ], + err: %i[child out], + &:read + ) + assert($?.success? && runner_output == "runner-loaded", + "expected sanitized environment to execute the correction runner, got #{runner_output.inspect}") end def assert_start_spawns_harness_through_validation_runner