fix(desktop): clarify partial transcript ranges - #3880
Conversation
d481f85 to
9eb99b9
Compare
ba2a115 to
efb4ff5
Compare
Generated-by: Maka
efb4ff5 to
6d33618
Compare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for making bounded transcript history clearer without bringing back full-history hydration.
The notice now explains when newer durable history exists, while “Return to latest” preserves the direct recovery path. Building the prompt rail from the existing landmark index also keeps navigation anchored to the current authority: non-resident prompts become loadable targets rather than a second copy of transcript state.
I verified exact head 424bb1a69e33607c7c603a7ad384967b62f9b535: the UI suite passes 237 tests, the transcript range-store suite passes 16 tests, and Desktop typecheck, changed-file Biome, git diff --check, and the hosted test check all pass. I found no P0–P3 issues and no code or tests that should be removed.
Review analysis was assisted by Codex; Astro-Han reviewed the result and owns this approval.
简体中文
谢谢在不恢复全历史加载的前提下,把有界 transcript 的状态解释清楚。
现在提示会明确说明仍有较新的持久化历史,“Return to latest”也保留了直接返回最新范围的路径。Prompt rail 继续以现有 landmark index 为唯一导航来源,未加载的 prompt 只是可加载目标,没有复制出第二份 transcript 状态。
我验证了精确 head 424bb1a69e33607c7c603a7ad384967b62f9b535:UI suite 237 项、transcript range-store suite 16 项全部通过;Desktop typecheck、改动文件的 Biome、git diff --check 和 GitHub test 也通过。没有发现 P0–P3 问题,也没有应删除的代码或测试。
本次审查分析由 Codex 协助;Astro-Han 审阅了结果并对本次 Approve 负责。
| onReturnToLatest, | ||
| }: TranscriptHistoryNoticeProps) { | ||
| return ( | ||
| <div |
There was a problem hiding this comment.
Thanks for making this state understandable. [P3, non-blocking] One visual suggestion: this is a navigation-position state rather than a warning, so the full-width bordered block feels a little heavier than necessary.
Could we make it a quiet, single-line Astryx composition inside the reading column, using HStack, secondary Text, and a small ghost Button? “Viewing earlier messages” with “Return to latest” should be enough. I would also avoid exposing “saved but not loaded” or teaching loaded residency through hollow rail ticks, since those are implementation details.
This does not need to block the PR.
简体中文
谢谢把这个状态解释清楚。[P3,非阻塞] 有一个视觉上的建议:这是对话浏览位置的状态,不是警告,所以现在通栏、描边的区块稍微有些重。
可以考虑在阅读栏内使用更轻的 Astryx 单行组合,例如 HStack、次要层级的 Text 和一个小号 ghost Button。“Viewing earlier messages”配合“Return to latest”就足够了。我也建议避免使用“已保存但尚未加载”或通过空心 rail 刻度让用户理解驻留状态,因为这些属于实现细节。
这个建议不需要阻塞当前 PR。
|
Thanks again for working through this. The current fix is correct for the architecture it is built on, and I do not think #3880 should grow into an architectural rewrite. While reviewing it, though, I noticed a deeper follow-up that seems worth capturing in a separate issue. The 512 KiB value is a range-fetch budget, not a conversation limit. Today, jumping outside the resident range makes Would you be interested in opening and taking an architecture issue for this? A possible direction would be:
That would let the user navigate one continuous logical transcript while memory and DOM usage remain bounded. Raising the 512 KiB budget would only move the threshold, whereas joining data paging to the existing virtual window would address the cause. The large-transcript fixture and range-store tests from this PR look like a useful starting point for that work. I think you are well positioned to take it on, and I would be happy to help review the design incrementally. This is follow-up architecture work, not a defect in #3880, so it does not change my approval. Review analysis was assisted by Codex; Astro-Han reviewed the data path and owns this feedback. 简体中文再次谢谢你把这个问题处理清楚。当前修复在既有架构下是正确的,我也不认为应该把 #3880 扩大成一次架构重写。 不过在审查过程中,我注意到一个值得单独记录的深层改进。512 KiB 是一次范围读取的预算,并不是对话历史的上限。现在跳转到驻留范围之外时, 你是否愿意开一个独立的架构 issue,并继续推进这个改进?可以考虑以下方向:
这样既能让用户在一条连续的逻辑 transcript 中导航,也能继续限制内存和 DOM 使用。单纯提高 512 KiB 只会把阈值向后移动,把数据分页和现有虚拟窗口真正接起来才是在解决根因。 当前 PR 中的大 transcript fixture 和 range-store 测试看起来很适合作为后续工作的起点。你已经梳理了不少相关行为,我觉得很适合继续推进;如果采用分阶段设计,我也愿意逐步协助审查。 这是后续的架构改进,不是 #3880 本身的缺陷,因此不会改变我的 Approve。 本次审查分析由 Codex 协助;Astro-Han 审阅了数据路径并对这些反馈负责。 |
Summary
Before / After
Before
After
Fixes #3875
Visual evidence
Production-faithful Storybook shell, captured at 1440 × 900 with the same bounded historical transcript state:
Verification
npm --workspace @maka/ui test— 235 tests passednode --test apps/desktop/dist/main/__tests__/desktop-transcript-range-store.test.js— 16 tests passednpm run buildnpm run typechecknpm run lintnpm run format:checkgit diff --checkThe regression coverage includes an eight-turn transcript larger than
DESKTOP_TRANSCRIPT_RANGE_MAX_BYTES, historical and intermediate prompt navigation, return to latest, renderer range bounds, accessible notice markup, and resident/unloaded prompt-rail semantics.AI use
Select exactly one:
Tool(s) and scope: Maka analyzed the issue, implemented the UI and regression coverage, and ran the listed validation commands.
Checklist
Does this PR entail a change in behavior?