Skip to content

Highlight a clicked user message even with the log panes closed #486

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iceljc merged 1 commit into from
Sep 2, 2026
+27 −2
Merged

Highlight a clicked user message even with the log panes closed #486

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions src/routes/chat/[agentId]/[conversationId]/chat-box.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1653,11 +1653,19 @@
}
}

/** @param {string} messageId */
/**
* Clicking a user message marks it as the one being inspected, and points the
* log panes at it. The highlight is unconditional — it is feedback that the
* click landed, and the message stays picked out whether or not the logs are
* on screen. Directing the panes only makes sense while they are open.
* @param {string} messageId
*/
function directToLog(messageId) {
if (!messageId || isLite || !isLoadPersistLog) return;
if (!messageId || isLite) return;

highlightedMsgId = messageId;
if (!isLoadPersistLog) return;

highlightStateLog(messageId);
autoScrollToTargetLog(messageId);
}
Expand All @@ -1682,6 +1690,9 @@

/** @param {string} messageId */
function autoScrollToTargetLog(messageId) {
// Tell a freshly opened log pane to stop pinning itself to the tail, or it
// would pull straight back down from the entry we are about to show.
window.dispatchEvent(new CustomEvent('persist-log:cancel-pin'));
Comment on lines +1693 to +1695

@qodo-code-review qodo-code-review Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Cancel event lost during load 🐞 Bug ≡ Correctness

autoScrollToTargetLog dispatches cancellation synchronously, but the persist-log listener is
installed only after both asynchronous log requests and a DOM tick complete. A click during that
interval can scroll an already-rendered target, then the later settling pin starts and drags the
pane back to the tail.
Agent Prompt 
## Issue description
The cancel-pin event can fire before the persist-log pane finishes asynchronous initialization and registers its listener. The later settling pin then starts despite the cancellation and can override the message-targeted scroll.

## Issue Context
PersistLog waits for both log requests and `tick()` before calling `pinToBottomWhileSettling()`, which currently owns listener registration. Register cancellation synchronously during component mounting and retain a cancellation flag so delayed initialization does not start the settling pin after an earlier event.

## Fix Focus Areas
- src/routes/chat/[agentId]/[conversationId]/chat-box.svelte[1692-1695]
- src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte[86-101]
- src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte[182-192]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

const contentLogWrapper = '.content-log-scrollbar';
const stateLogWrapper = '.conv-state-log-scrollbar';
const elements = [];
Expand Down
14 changes: 14 additions & 0 deletions src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
const contentLogTab = 1;
const conversationStateLogTab = 2;
const conversationId = page.params.conversationId;
/** Fired by chat-box when it scrolls a pane to a specific log entry. */
const CANCEL_PIN_EVENT = 'persist-log:cancel-pin';
const utcNow = moment.utc().toDate();

const scrollbarElements = [
Expand Down Expand Up @@ -178,6 +180,17 @@
* @param {number} timeoutMs
*/
function pinToBottomWhileSettling(timeoutMs = 3000) {
/** @type {(() => void)[]} */
const stops = [];
/*
* Opening the panes to look at one message races the settling pin: the pin
* would drag the pane back to the tail moments after the jump. A scroll
* aimed at a specific entry cancels the pin, the same way a wheel does.
*/
const cancelPin = () => stops.forEach(stop => stop());
window.addEventListener(CANCEL_PIN_EVENT, cancelPin);
stops.push(() => window.removeEventListener(CANCEL_PIN_EVENT, cancelPin));
Comment on lines +190 to +192

@qodo-code-review qodo-code-review Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation recommended

2. Cancel listener leaks after pin 🐞 Bug ☼ Reliability

pinToBottomWhileSettling registers a window listener whose removal callback runs only when a
future cancel event iterates stops; normal timeouts, user interaction, and component teardown
never remove it. Repeatedly opening and closing logs therefore retains stale component and scrollbar
closures indefinitely.
Agent Prompt 
## Issue description
The global `persist-log:cancel-pin` listener remains registered after the settling period ends and after PersistLog is destroyed. Each pane mount can therefore leak another listener and its captured DOM and scrollbar state.

## Issue Context
Local `stop` callbacks disconnect observers and clear timers, but they do not remove the window listener. Ensure listener removal occurs when all pins stop, when cancellation occurs, and during component teardown, including teardown before asynchronous initialization completes.

## Fix Focus Areas
- src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte[86-101]
- src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte[182-192]
- src/routes/chat/[agentId]/[conversationId]/persist-log/persist-log.svelte[205-218]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


scrollbars.forEach(scrollbar => {
if (!scrollbar) return;

Expand All @@ -202,6 +215,7 @@
viewport.addEventListener('pointerdown', stop);
viewport.addEventListener('keydown', stop);
timer = setTimeout(stop, timeoutMs);
stops.push(stop);
});
}

Expand Down
Loading