From e8732e234d4b717e1308d75639cc1cf0ca0a5c7c Mon Sep 17 00:00:00 2001 From: Hamhire Hu Date: Tue, 8 Sep 2026 19:21:25 +0800 Subject: [PATCH] fix(chat): anchor the commit divider to the boundary, not to the end of the pane The trailing commit divider rendered after everything in the timeline, so it was not marking a position -- it was tracking the bottom. Every message, thinking step or queued run that arrived pushed it further down, and a boundary that moves is worse than no boundary: it reads as if the commit itself keeps happening later. The boundary belongs immediately after the last run that reviewed the previous commit. Everything that arrives afterwards belongs below it, which is also what the divider means. So it now anchors before whatever first followed that run, and only falls back to the end in the one case where nothing follows it yet -- where the two positions coincide anyway. Placement moves into computeCommitDividers, a pure function: it is the part with the actual reasoning, and it was previously inlined in a component with no way to exercise it. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 1 + CHANGELOG.zh-CN.md | 1 + .../src/components/features/chat/ChatPane.tsx | 34 +++++-------- .../features/chat/utils/commit-dividers.ts | 49 +++++++++++++++++++ 4 files changed, 62 insertions(+), 23 deletions(-) create mode 100644 apps/desktop/src/renderer/src/components/features/chat/utils/commit-dividers.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f23fa34..7642b774 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ and the versioning follows [Semantic Versioning](https://semver.org/). ### 🔧 Fixed +- The commit divider in the review timeline now stays where the commit boundary actually is, instead of sliding further down every time a new message appears below it. - The file list in a generated PR description now shows the real number of added and removed lines per file, instead of `+-1/--1`. - Links in that file list now open the file instead of pointing at a non-existent line, so clicking through works. - A PR description ending in a git merge tail (`# Conflicts:` and the file lines under it) no longer breaks up the generated description — those lines are part of the quoted description, not headings of their own. diff --git a/CHANGELOG.zh-CN.md b/CHANGELOG.zh-CN.md index 008f7183..8f0c9dc8 100644 --- a/CHANGELOG.zh-CN.md +++ b/CHANGELOG.zh-CN.md @@ -17,6 +17,7 @@ ### 🔧 修复 +- 评审时间线中的提交分割线现在固定在提交边界的真实位置，不再因为下方出现新消息而一路下移。 - 生成的 PR 描述中，文件清单现在显示每个文件真实的增删行数，不再是 `+-1/--1`。 - 该清单中的链接现在指向文件本身，而非一个不存在的行号，点击可正常跳转。 - PR 描述末尾带有 git 合并残留（`# Conflicts:` 及其下的文件行）时，生成的描述不再被切碎——那些行属于被引用的描述正文，而不是标题。 diff --git a/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx b/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx index 8fcd5b69..df25119c 100644 --- a/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx +++ b/apps/desktop/src/renderer/src/components/features/chat/ChatPane.tsx @@ -27,6 +27,7 @@ import { useChatTimeline } from './hooks/useChatTimeline'; import { AgentStepRow, ThinkingLive } from './components/AgentStep'; import { ChatEmpty } from './components/ChatEmpty'; import { CommitDivider } from './components/CommitDivider'; +import { computeCommitDividers } from './utils/commit-dividers'; import { ChatInputBar } from './components/ChatInputBar'; import { ConversationMessage } from './components/ConversationMessage'; import { PlanPanel } from './components/PlanPanel'; @@ -244,26 +245,12 @@ export function ChatPane({ prLocalId, }); - // Commit dividers: mark every point in the run timeline where the reviewed commit changes, so the boundary persists - // rather than vanishing once the new code is reviewed. Two cases: - // - between two consecutive runs whose headSha differs → a divider *before* the newer run (a durable boundary - // between the old-commit runs above and the new-commit runs below); - // - a trailing divider at the bottom when the current PR head has advanced past the last run's commit (covers a new - // commit that hasn't been reviewed yet — including while a run against it is still in flight). - // The timeline is ascending by start time; only runs that recorded a headSha participate (pre-feature runs are skipped). - const commitDividers = useMemo(() => { - const before = new Map(); // timeline entry.key → the newer headSha to render a divider before it - let prevSha: string | undefined; - for (const entry of timeline) { - const sha = entry.run?.headSha; - if (!sha) continue; - if (prevSha && sha !== prevSha) before.set(entry.key, sha); - prevSha = sha; - } - const head = pr?.sourceRef.sha; - const bottom = head && prevSha && head !== prevSha ? head : null; - return { before, bottom }; - }, [timeline, pr?.sourceRef.sha]); + // Commit dividers: every point in the timeline where the reviewed commit changes (see computeCommitDividers, which + // also explains why the trailing boundary anchors after the last run rather than at the end of the pane). + const commitDividers = useMemo( + () => computeCommitDividers(timeline, pr?.sourceRef.sha), + [timeline, pr?.sourceRef.sha], + ); // Commit messages for divider tooltips: fetch the PR's commits (main-cached; keyed on head sha so it refreshes when // the head advances) into a sha → message map. Empty until loaded / on failure (the tooltip falls back to the short sha). @@ -451,9 +438,10 @@ export function ChatPane({ ) : null, )} - {/* Bottom commit divider: the PR head advanced past the last run's commit and no run against it exists yet - (a new commit not reviewed yet, including while a run against it is still in flight). Once such a run - completes, the boundary instead renders between the old and new runs above (see commitDividers.before). */} + {/* Trailing commit divider, for the one case with no entry to precede: the head advanced past the last run's + commit and nothing has landed after that run yet. As soon as anything does — a message, a step, a queued + run — the boundary moves into `before` and anchors there, so it stays put instead of being pushed down by + each new bubble (see computeCommitDividers). */} {commitDividers.bottom && ( ; + /** headSha for a divider after everything, used only when the boundary has no entry to precede. */ + bottom: string | null; +} + +/** + * Place the commit boundaries in the chat timeline: every point where the reviewed commit changes, so the boundary + * persists rather than vanishing once the new code has been reviewed. + * + * - **Between runs**: two consecutive runs with different `headSha` → a boundary before the newer one, separating the + * old-commit runs above from the new-commit ones below. + * - **After the newest run**: the PR head has advanced past the last run's commit and nothing has reviewed it yet. + * The boundary belongs immediately after that last run — **not at the end of the timeline**. Those differ as soon as + * anything else lands (a message, a thinking step, a queued run), and anchoring to the end made the divider drift + * down the pane on every new bubble, as if the boundary itself kept moving. A boundary marks a point in history; it + * has to stay where that point is, with everything that came after it below. + * + * The timeline is ascending by start time; only runs that recorded a `headSha` participate (runs from before that field + * existed are skipped, which is why the last *run* and the last *sha-bearing run* are tracked as the same cursor). + */ +export function computeCommitDividers( + timeline: readonly TimelineEntry[], + headSha: string | undefined, +): CommitDividers { + const before = new Map(); + let prevSha: string | undefined; + let lastShaIdx = -1; + timeline.forEach((entry, i) => { + const sha = entry.run?.headSha; + if (!sha) return; + if (prevSha && sha !== prevSha) before.set(entry.key, sha); + prevSha = sha; + lastShaIdx = i; + }); + // No run has recorded a sha, or the head is the one already reviewed → no trailing boundary. + if (!headSha || !prevSha || headSha === prevSha) return { before, bottom: null }; + // Anchor the boundary before whatever first followed that last run. Nothing follows it yet → fall back to the end, + // which is then the same position (and stops being a moving target as soon as an entry appears there). + const next = timeline[lastShaIdx + 1]; + if (next) { + before.set(next.key, headSha); + return { before, bottom: null }; + } + return { before, bottom: headSha }; +}