Skip to content

feat(ui): add ui.history.collapsePreviewCount to show last N turns when resuming collapsed sessions - #5848

Merged
wenshao merged 5 commits into
QwenLM:mainfrom
mvanhorn:feat/5759-collapse-preview-count
Jun 30, 2026
Merged

feat(ui): add ui.history.collapsePreviewCount to show last N turns when resuming collapsed sessions#5848
wenshao merged 5 commits into
QwenLM:mainfrom
mvanhorn:feat/5759-collapse-preview-count

Conversation

@mvanhorn

Copy link
Copy Markdown
Contributor

What this PR does

Adds a new ui.history.collapsePreviewCount setting (number, default 0) that keeps the most recent N user turns visible while collapsing the rest of the restored transcript when resuming a session with ui.history.collapseOnResume enabled. A user turn is a user prompt plus its assistant response and associated tool/thinking items. The summary line's hidden count reflects only the items that were actually collapsed, and the summary is omitted when nothing is hidden.

Why it's needed

Today, when ui.history.collapseOnResume is enabled, resuming a session hides the entire restored transcript and shows only a one-line summary. That avoids the slow full-history redraw, but it makes it impossible to see where you left off without running /history expand-now, which re-triggers the same slow redraw. As discussed in #5759, the resume-collapse path is currently all-or-nothing. collapsePreviewCount makes the collapse partial so the last N turns stay readable while older context is summarized.

Semantics: 0 (default) collapses all restored history and the summary shows the full count (unchanged behavior); N (> 0) keeps the last N user turns visible, collapses earlier items, and the summary counts only the hidden items (if N is greater than or equal to the number of turns, all turns stay visible and no summary is appended); -1 shows all restored history (equivalent to collapseOnResume: false); when collapseOnResume is false, collapsePreviewCount has no effect.

Reviewer Test Plan

This is a non–user-visible logic change to the resume-history policy plus a new setting, covered by unit tests.

How to verify

applyCollapsePolicyAndSummary now accepts a collapsePreviewCount argument and is exercised by new unit tests covering: default-collapse (all hidden, full summary count), partial preview (last N turns visible, summary counts only hidden items), preview larger than the turn count (all visible, no summary), -1 (all visible, no summary), collapseOnResume: false (raw items unchanged), and empty history (no summary, no crash). The setting is read at the three resume call sites (AppContainer.tsx, useResumeCommand.ts, useBranchCommand.ts) and threaded through.

Reviewer commands:

  • npx vitest run src/ui/utils/resumeHistoryUtils.test.ts (24 tests pass)
  • npx vitest run src/ui/hooks/useResumeCommand.test.ts src/ui/hooks/useBranchCommand.test.ts (28 tests pass)
  • npm run typecheck, npm run lint, npm run format — clean.

Evidence (Before & After)

N/A (non–user-visible logic change to the resume-history policy; verified via unit tests and typecheck output above).

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

N/A — unit tests only (vitest, tsc --noEmit).

Risk & Scope

  • Main risk or tradeoff: turn boundaries are detected by MessageType.USER items; the default 0 path is preserved byte-for-byte so existing collapse-on-resume behavior is unchanged.
  • Not validated / out of scope: no change to /history expand-now or the rewind/turn-mapping paths; canonical history is untouched (only the suppressOnRestore display flag is varied).
  • Breaking changes / migration notes: none; new setting defaults to 0, which is the current behavior.

Linked Issues

Fixes #5759

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI failing (Test (ubuntu-latest, Node 22.x)).

Two inline suggestions below. Overall this is a clean, well-scoped change (+130/-4, 7 files) with 52 tests passing locally, deterministic analysis clean (tsc/eslint 0 findings), and good backward-compat preservation at the default value.

showInDialog: false,
},
collapsePreviewCount: {
type: 'number',

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Schema declares type: 'number' but the boundary algorithm in applyCollapsePolicyAndSummary requires an integer — it uses strict equality (userTurnCount === collapsePreviewCount) against an integer counter. A float like 2.5 silently produces incorrect behavior (the loop never matches, falls through to unexpected collapse).

Other integer-valued settings in this same file use jsonSchemaOverride to enforce integer constraints (e.g., quorumSize at line 2288, stopHookBlockCap at line 2629, fileHistoryRetentionDays at line 1568).

Suggested change
type: 'number',
collapsePreviewCount: {
type: 'number',
label: 'Collapse Preview Count',
category: 'UI',
requiresRestart: false,
default: 0,
description:
'Number of most recent user turns to keep visible when collapsing history on resume. 0 collapses all restored history by default; -1 shows all restored history.',
jsonSchemaOverride: {
type: 'integer',
minimum: -1,
},
showInDialog: false,
},

— qwen3.7-max via Qwen Code /review

}),
);
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The userTurnCount < collapsePreviewCount fallback branch at resumeHistoryUtils.ts:586-588 is never exercised by the current test suite. The existing "covers all user turns" test uses previewCount=3 with exactly 3 user turns — the loop hits break (3 === 3), so boundary is set to 0 inside the loop, not via the post-loop fallback.

A test with previewCount exceeding the actual turn count would cover this distinct code path:

it('shows all items without a summary when preview count exceeds user turns', () => {
  const rawItems = makeItems(); // 3 user turns
  const result = applyCollapsePolicyAndSummary(rawItems, true, 5);
  expect(result).toEqual(rawItems);
  result.forEach(expectVisible);
});

— qwen3.7-max via Qwen Code /review

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

⚠️ Downgraded from Approve to Comment: CI failing (Test (ubuntu-latest, Node 22.x)).

— qwen3.7-max via Qwen Code /review

if (userTurnCount === collapsePreviewCount) {
boundary = i;
break;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] userTurnCount === collapsePreviewCount uses strict equality against an always-integer counter. If collapsePreviewCount is a float (e.g. 1.5 — schema declares type: 'number' without integer constraints), the comparison never matches, the backward walk exhausts all items, and boundary stays at rawItems.length, collapsing everything instead of keeping ~1 turn visible.

Suggested change
}
if (userTurnCount >= collapsePreviewCount) {

Using >= makes the loop stop at the Nth user turn for any N (integer or fractional) and makes the post-loop userTurnCount < collapsePreviewCount fallback consistent.

— qwen3.7-max via Qwen Code /review

);
});

it('keeps the most recent N user turns visible and summarizes only hidden items', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] No test covers collapsePreviewCount=1, the most common non-zero value users will configure. This is a meaningful boundary — it should keep exactly the last user turn (and its assistant response) visible while collapsing everything else. Currently tested values: 0, 2, 3, -1.

  it('keeps only the last user turn visible when previewCount is 1', () => {
    const result = applyCollapsePolicyAndSummary(makeItems(), true, 1);

    expect(result).toHaveLength(7);
    result.slice(0, 4).forEach(expectSuppressed);
    result.slice(4, 6).forEach(expectVisible);
    expect(result[6]).toEqual(
      expect.objectContaining({
        text: expect.stringContaining('4 messages hidden'),
        display: { kind: 'collapse-summary' },
      }),
    );
  });

— qwen3.7-max via Qwen Code /review

wenshao
wenshao previously approved these changes Jun 25, 2026

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed at same SHA. No new issues found beyond the 4 suggestions from the prior review (integer schema enforcement, test coverage for boundary branches and collapsePreviewCount=1). Core boundary algorithm is correct across all edge cases. All 52 tests pass, CI green (30/30). LGTM! ✅

— qwen3.7-max via Qwen Code /review

@wenshao

wenshao commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

✅ Maintainer verification — local build test + CI-failure diagnosis

Built and ran the PR locally at head 07336440e. Bottom line: the logic is correct and fully tested locally — the red CI is a real but trivial blocker (a generated file wasn't regenerated), and it caused the unit tests to be skipped in CI. One command fixes it.

1. The failing CI is real — diagnosed & reproduced 🔴

The red check is Test (ubuntu-latest) › step "Check settings schema is up-to-date", which runs npm run generate:settings-schema then fails if packages/vscode-ide-companion/schemas/settings.schema.json changes.

  • Root cause: settingsSchema.ts gained ui.history.collapsePreviewCount, but the generated settings.schema.json was never regenerated/committed. Confirmed: collapsePreviewCount appears 0 times in the committed schema (at the feature tip and at the merge head — so the merge didn't drop it; it was simply never regenerated), while the source schema has it.
  • Reproduced locally — regenerating produces exactly this (the only diff):
"collapsePreviewCount": {
  "description": "Number of most recent user turns to keep visible when collapsing history on resume. 0 collapses all restored history by default; -1 shows all restored history.",
  "type": "number",
  "default": 0
}
  • Knock-on effect: this step runs before "Run tests", so the PR's unit tests were skipped in this CI run — the "tests pass" evidence in the description was local-only, never validated by CI.
  • Fix (one line, by the contributor):
    npm run generate:settings-schema
    git add packages/vscode-ide-companion/schemas/settings.schema.json && git commit
    

2. Logic + tests — verified locally (exactly what CI skipped) ✅

Check Result
resumeHistoryUtils.test.ts 24 / 24
useResumeCommand.test.ts (11) + useBranchCommand.test.ts (17) 28 / 28
My own independent suite (fresh data, not the author's) 10 / 10
npm run typecheck (all 5 packages) exit 0
eslint (changed files) exit 0

My independent suite re-derives the documented semantics with harder data than the author's clean user/assistant pairs — interleaved TOOL_GROUP items, a leading INFO banner, and out-of-range counts:

  • count=0 collapses everything, summary counts all items, and is byte-for-byte identical to omitting the arg (back-compat).
  • count=1 keeps only the last turn visible (ids [16,17]), hides 6; count=2 keeps the last two turns ([13..17]), hides 3 — boundary correctly lands on MessageType.USER items even with tool items interleaved.
  • count ≥ turns (3 / 4 / 999) → all visible, no summary; -1 and collapseOnResume:false → the same array reference; empty history → no crash, no summary.
  • Edge: a leading INFO banner before the first turn is still collapsed when count == turns (summary count 1) — worth knowing, harmless.

3. Integration wiring — audited ✅

All three resume entry points (AppContainer.tsx, useResumeCommand.ts, useBranchCommand.ts) read settings.merged.ui?.history?.collapsePreviewCount ?? 0, thread it into applyCollapsePolicyAndSummary, and the two hooks correctly add it to their useCallback dependency arrays. typecheck passing proves the typed settings path is real (not a typo).

4. One non-blocking nit (consistent with my earlier review)

The setting is type: "number" with no integer / minimum constraint, so out-of-range values don't crash but behave surprisingly: -2 silently collapses everything (like 0), and a non-integer like 2.5 is never matched by the turn counter. The "enforce integer / >= -1" hardening I suggested earlier wasn't adopted. Cosmetic robustness only — not a merge blocker.

Merge state

mergeable = MERGEABLE; BLOCKED solely by the failing schema check. The logic is merge-ready and independently verified (62 tests green); the only action needed is for the contributor to run npm run generate:settings-schema and commit the result — after which CI advances to (and should pass) the unit tests.

🇨🇳 中文版(点击展开)

✅ 维护者验证 —— 本地构建测试 + CI 失败诊断

在 head 07336440e 本地构建并运行了该 PR。结论:逻辑正确、本地全测通过 —— 红的 CI 是个真实但很小的阻塞(一个生成文件没重新生成),而且它导致单测在 CI 里被跳过。 一条命令即可修复。

1. CI 失败是真的 —— 已诊断并复现 🔴

红的检查是 Test (ubuntu-latest) › 步骤 "Check settings schema is up-to-date",它先跑 npm run generate:settings-schema,若 packages/vscode-ide-companion/schemas/settings.schema.json 有变化就失败。

  • 根因: settingsSchema.ts 新增了 ui.history.collapsePreviewCount,但生成的 settings.schema.json 从没重新生成/提交。已确认:已提交的 schema 里 collapsePreviewCount 出现 0 次(在特性分支尖端和合并 head 都是 0 —— 所以不是 merge 弄丢的,而是压根没重生成),而源 schema 里有。
  • 本地已复现 —— 重新生成后恰好产生这一段(唯一的 diff):
"collapsePreviewCount": {
  "description": "Number of most recent user turns to keep visible when collapsing history on resume. 0 collapses all restored history by default; -1 shows all restored history.",
  "type": "number",
  "default": 0
}
  • 连带影响: 这一步在 "Run tests" 之前,所以本次 CI 的单测被 skipped —— 描述里的"测试通过"只是本地结果,CI 从没验证过。
  • 修复(贡献者一行):
    npm run generate:settings-schema
    git add packages/vscode-ide-companion/schemas/settings.schema.json && git commit
    

2. 逻辑 + 测试 —— 本地已验证(正是 CI 跳过的部分)✅

检查 结果
resumeHistoryUtils.test.ts 24 / 24
useResumeCommand.test.ts (11) + useBranchCommand.test.ts (17) 28 / 28
我自己独立写的用例(全新数据,非作者的) 10 / 10
npm run typecheck(全部 5 个包) exit 0
eslint(改动文件) exit 0

我的独立用例用比作者更刁钻的数据重新推导了文档语义 —— 夹杂 TOOL_GROUP 项、开头有 INFO banner、以及越界的 count:

  • count=0 折叠全部、摘要计入所有项,且与不传该参数逐字节一致(向后兼容)。
  • count=1 只保留最后一轮可见(id [16,17]),隐藏 6 项;count=2 保留最后两轮([13..17]),隐藏 3 项 —— 即使夹着 tool 项,边界也正确落在 MessageType.USER 上。
  • count ≥ 轮数(3 / 4 / 999)→ 全可见、无摘要-1collapseOnResume:false返回同一数组引用;空历史 → 不崩、无摘要。
  • 边界: 首轮之前的 INFO banner 在 count == 轮数 时仍被折叠(摘要计 1)—— 值得知道,无害。

3. 接线(集成)—— 已审计 ✅

三个 resume 入口(AppContainer.tsxuseResumeCommand.tsuseBranchCommand.ts)都读 settings.merged.ui?.history?.collapsePreviewCount ?? 0,传入 applyCollapsePolicyAndSummary,且两个 hook 正确把它加进了 useCallback 依赖数组。typecheck 通过即证明这个带类型的 settings 路径是真的(不是拼写错误)。

4. 一个不阻塞的小问题(与我之前的 review 一致)

该设置是 type: "number",没有 integer / minimum 约束,所以越界值不崩但行为意外:-2 会静默折叠全部(等同 0),2.5 这类非整数永远匹配不上轮次计数。我之前建议的"强制整数 / >= -1"加固没被采纳。纯属健壮性,不是合并阻塞

合并状态

mergeable = MERGEABLEBLOCKED 仅因 schema 检查失败。逻辑已可合并、且独立验证通过(62 个测试全绿);唯一要做的是贡献者跑一次 npm run generate:settings-schema 并提交结果 —— 之后 CI 才会推进到(并应当通过)单测。

@wenshao

wenshao commented Jun 26, 2026

Copy link
Copy Markdown
Collaborator

Build failure root cause: generated settings.schema.json is out of date

This isn't a compile error — the failing step is "Check settings schema is up-to-date" in the Test (ubuntu-latest, Node 22.x) job (it runs after build/test, which is why the job took ~6 min before failing):

Error: settings.schema.json is out of date.
Please run: npm run generate:settings-schema
Then commit the updated schema file.

Root cause

packages/vscode-ide-companion/schemas/settings.schema.json is a generated artifact derived from the setting definitions in packages/cli/src/config/settingsSchema.ts. This PR adds the new ui.history.collapsePreviewCount setting to settingsSchema.ts, but the regenerated schema JSON was never committed — it's not in the diff.

The CI step regenerates the schema and then runs git status --porcelain on it; since regeneration produces a diff against the committed file, it exits 1. The diff it would have produced is exactly the block printed in the log:

+            },
+            "collapsePreviewCount": {
+              "description": "Number of most recent user turns to keep visible when collapsing history on resume. 0 collapses all restored history by default; -1 shows all restored history.",
+              "type": "number",
+              "default": 0
+            }

Fix

npm run generate:settings-schema
git add packages/vscode-ide-companion/schemas/settings.schema.json
git commit

That regenerates and commits the derived schema so it matches settingsSchema.ts. No code change is needed — the source definition is already correct.

Note: this is an ubuntu-only static check; the macOS/Windows legs only build + test, so they were skipped rather than catching it.

中文版

构建失败根因:生成的 settings.schema.json 落后于源定义

这不是编译错误——失败的步骤是 Test (ubuntu-latest, Node 22.x) job 里的 "Check settings schema is up-to-date"(它在 build/test 之后跑,所以 job 失败前耗时约 6 分钟):

Error: settings.schema.json is out of date.
Please run: npm run generate:settings-schema
Then commit the updated schema file.

根因

packages/vscode-ide-companion/schemas/settings.schema.json 是一个生成产物,由 packages/cli/src/config/settingsSchema.ts 里的 setting 定义派生而来。本 PR 在 settingsSchema.ts 中新增了 ui.history.collapsePreviewCount 配置项,但重新生成的 schema JSON 从未被提交——它不在 diff 里

CI 步骤会重新生成 schema,然后对该文件跑 git status --porcelain;由于重新生成的结果与仓库里已提交的版本有差异,于是 exit 1。它本该产生的 diff 正是日志里打印的这一段:

+            },
+            "collapsePreviewCount": {
+              "description": "Number of most recent user turns to keep visible when collapsing history on resume. 0 collapses all restored history by default; -1 shows all restored history.",
+              "type": "number",
+              "default": 0
+            }

修复

npm run generate:settings-schema
git add packages/vscode-ide-companion/schemas/settings.schema.json
git commit

重新生成并提交这个派生 schema,使其与 settingsSchema.ts 保持一致。无需改任何代码——源定义本身已经正确。

注:这是一个 ubuntu-only 的静态检查;macOS/Windows 两条 leg 只 build + test,所以它们被 skip 而非由它们捕获此问题。

@wenshao
wenshao dismissed their stale review June 26, 2026 22:26

build error

@doudouOUC doudouOUC left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Integration test gap: The three call sites (AppContainer.tsx, useResumeCommand.ts, useBranchCommand.ts) all wire collapsePreviewCount from settings through to applyCollapsePolicyAndSummary(), but none of the integration tests exercise this path with a non-zero collapsePreviewCount. The unit tests in resumeHistoryUtils.test.ts cover the function logic thoroughly, but the end-to-end settings-to-function plumbing is untested. If a developer accidentally omits the parameter at a call site or introduces a typo in the settings key, no test would catch it.

expandCollapsedHistory test gap: No test exercises expandCollapsedHistory with the mixed-shape history that collapsePreviewCount > 0 produces (some items with suppressOnRestore: true, some without, plus a collapse-summary sentinel). The existing tests only cover fully-collapsed or no-collapse inputs.

— qwen3.7-max via Qwen Code /review

@@ -496,6 +497,88 @@ describe('resumeHistoryUtils', () => {
});
});

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Good unit test coverage for applyCollapsePolicyAndSummary itself, but the three call sites (AppContainer.tsx, useResumeCommand.ts, useBranchCommand.ts) that wire settings.merged.ui?.history?.collapsePreviewCount through to this function are not integration-tested with a non-zero collapsePreviewCount. If a developer accidentally omits the parameter at a call site or introduces a typo in the settings key, no test would catch it.

Consider adding at least one integration test (e.g., in useResumeCommand.test.ts) that sets collapsePreviewCount: 2 in the settings mock and asserts the loaded history has both suppressed and visible items plus a summary with the correct hidden count.

— qwen3.7-max via Qwen Code /review

});

describe('stripSuppressOnRestore', () => {
it('returns item unchanged when display is undefined', () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Nice to have] No test exercises expandCollapsedHistory with the mixed-shape history that collapsePreviewCount > 0 produces — some items with suppressOnRestore: true (hidden prefix), some without (visible preview tail), and a collapse-summary sentinel. The existing expandCollapsedHistory tests only cover fully-collapsed or no-collapse inputs.

Consider adding a test case that constructs a partially-collapsed history (e.g., 2 suppressed + 4 visible + 1 collapse-summary) and asserts that expandCollapsedHistory returns all 6 original items with suppressOnRestore stripped and the collapse-summary removed.

— qwen3.7-max via Qwen Code /review

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed at c19c3c0. Core boundary algorithm is correct — traced through all edge cases (default 0, partial preview, -1 sentinel, empty history, fewer turns than requested). Tests pass locally (24 tests in resumeHistoryUtils.test.ts). Deterministic analysis clean (tsc, eslint: 0 findings). CI all green (12/12 checks).

Prior suggestions (integer schema, test coverage gaps) remain open for the author's consideration.

— qwen3.7-max via Qwen Code /review

if (!collapseOnResume) return rawItems;
if (collapsePreviewCount === -1) return rawItems;

let boundary = rawItems.length;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Negative values other than -1 silently fall through to full collapse

The === -1 check only catches the documented sentinel. Any other negative value (e.g., -2, -100) bypasses both the early-return here and the > 0 branch below, leaving boundary = rawItems.length — which collapses everything with a summary. The schema has no minimum constraint, so this is reachable from settings.

Suggested change
let boundary = rawItems.length;
if (collapsePreviewCount < 0) return rawItems;

— bailian/glm-5.2 via Qwen Code /review

});

it('shows all items without a summary when preview count is -1', () => {
const rawItems = makeItems();

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] Test doesn't exercise leading non-USER items (common in real sessions)

The makeItems() fixture starts with a USER message. With collapsePreviewCount=3 and 3 user turns, boundary lands at index 0, so no items are hidden. But real sessions frequently start with INFO/system messages. With a leading INFO item, the same inputs produce boundary > 0 — items ARE hidden and a summary IS appended, contradicting the test name's implication.

Consider adding:

it('still hides leading non-user items when preview count covers all user turns', () => {
  const items = [
    { id: 0, type: MessageType.INFO, text: 'system context' },
    ...makeItems(),
  ] as HistoryItem[];
  const result = applyCollapsePolicyAndSummary(items, true, 3);
  expect(result[0].display).toEqual(
    expect.objectContaining({ suppressOnRestore: true }),
  );
  expect(result).toHaveLength(8);
  expect(result[7].text).toContain('1 messages hidden');
});

— bailian/glm-5.2 via Qwen Code /review

@wenshao

wenshao commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @mvanhorn!

Template looks good ✓ (all required headings present, Linked Issues references #5759)

On direction: this solves a real pain point. When collapseOnResume is enabled, the all-or-nothing collapse makes it impossible to see where you left off without triggering the slow full-history redraw via /history expand-now. Adding a collapsePreviewCount to keep the last N turns visible is a natural, proportional fix. Issue #5759 confirms the user demand. Well within the UI/session-management scope of the project.

On approach: the scope feels right — one new setting, one function signature change, three call-site wiring updates. No unrelated edits, no drive-by refactors. The backward-scan for MessageType.USER boundaries is the simplest correct approach, and the default 0 preserves existing behavior byte-for-byte. One minor observation: the setting is type: "number" with no integer/minimum constraint, so values like -2 or 2.5 produce surprising-but-harmless behavior (silently collapse all / never match). Not a blocker, but worth noting for future hardening.

Moving on to code review. 🔍

中文说明

感谢贡献,@mvanhorn

模板完整 ✓(所有必需标题均存在,关联 Issue 引用了 #5759

方向:这解决了一个真实的痛点。启用 collapseOnResume 时,全有或全无的折叠方式让人无法看到上次离开的位置,除非通过 /history expand-now 触发缓慢的完整历史重绘。添加 collapsePreviewCount 来保留最后 N 轮可见是一个自然、适度的修复。Issue #5759 确认了用户需求,完全在项目 UI/会话管理的范围内。

方案:范围合理——一个新设置、一个函数签名变更、三个调用点的接线更新。无无关改动,无顺手重构。从后向前扫描 MessageType.USER 边界是最简单的正确方法,默认值 0 逐字节保留现有行为。一个小观察:设置是 type: "number",没有 integer/minimum 约束,所以 -22.5 等值会产生意外但无害的行为(静默全部折叠/永远不匹配)。不阻塞,但值得注意以备将来加固。

进入代码审查 🔍

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code Review

The core logic change is clean and correct. applyCollapsePolicyAndSummary now accepts collapsePreviewCount (default 0), scans backward for MessageType.USER boundaries to determine the split point, and only applies applyResumeDisplayPolicy to the hidden portion. The visible tail is passed through untouched. The summary count correctly uses the boundary index (number of hidden items), not rawItems.length.

Key observations:

  • Backward compatibility preserved: when collapsePreviewCount is 0 (or omitted), the loop is skipped entirely and boundary = rawItems.length — identical to the prior code path.
  • Edge cases handled: empty history returns [] without crash; collapsePreviewCount >= turnCount sets boundary = 0 and skips the summary entirely; -1 short-circuits to return rawItems directly.
  • No reuse concern: the backward-scan logic is specific to turn-boundary detection and doesn't duplicate any existing utility.
  • Wiring correct: all three call sites (AppContainer.tsx, useResumeCommand.ts, useBranchCommand.ts) read the setting with ?? 0 fallback and thread it through. Both hooks add it to their useCallback dependency arrays.

No correctness bugs, security issues, or structural violations found.

Test Results

Unit tests (all pass):

  • resumeHistoryUtils.test.ts: 24 / 24
  • useResumeCommand.test.ts (11) + useBranchCommand.test.ts (17): 28 / 28
  • npm run typecheck (all 6 packages): exit 0 ✓
  • npm run lint: exit 0 ✓
  • npm run generate:settings-schema: no diff (schema is up-to-date — previous CI blocker resolved) ✓

Real-Scenario Testing (tmux)

Created a 2-turn session ("say hello" → "now say goodbye"), then resumed with different collapsePreviewCount values.

collapsePreviewCount=1 (partial preview)

  ┌──────────────────────────────────────────────────────────────────────────┐
  │ >_ Qwen Code (vdev)                                                      │
  │                                                                          │
  │ API Key | qwen3.7-max (/model to change)                                 │
  │ ~/actions-runner-4/_work/qwen-code/qwen-code                             │
  └──────────────────────────────────────────────────────────────────────────┘
  Tips: Type / to open the command popup; Tab autocompletes slash commands and
   saved prompts.

  > now say goodbye

  ✦ Goodbye! Feel free to reach out whenever you need help.
  ● History collapsed: 2 messages hidden. Use /history expand-now to show.

Result: Last turn visible ("now say goodbye" + response), first turn collapsed, summary shows "2 messages hidden". ✓

collapsePreviewCount=0 (default, all collapsed)

  ┌──────────────────────────────────────────────────────────────────────────┐
  │ >_ Qwen Code (vdev)                                                      │
  │                                                                          │
  │ API Key | qwen3.7-max (/model to change)                                 │
  │ ~/actions-runner-4/_work/qwen-code/qwen-code                             │
  └──────────────────────────────────────────────────────────────────────────┘
  Tips: Add a QWEN.md file to give Qwen Code persistent project context.
  ● History collapsed: 7 messages hidden. Use /history expand-now to show.

Result: All history collapsed, no turns previewed, summary shows "7 messages hidden" (includes the extra turn added during the accidental interactive test). Default behavior unchanged. ✓

中文说明

代码审查

核心逻辑变更干净且正确。applyCollapsePolicyAndSummary 现在接受 collapsePreviewCount(默认 0),从后向前扫描 MessageType.USER 边界以确定分割点,并仅对隐藏部分应用 applyResumeDisplayPolicy。可见尾部原样传递。摘要计数正确使用 boundary 索引(隐藏项数),而非 rawItems.length

关键观察:

  • 向后兼容:当 collapsePreviewCount0(或省略)时,循环完全跳过,boundary = rawItems.length —— 与之前的代码路径完全一致。
  • 边界情况已处理:空历史返回 [] 不崩溃;collapsePreviewCount >= turnCount 设置 boundary = 0 并跳过摘要;-1 直接返回 rawItems
  • 无复用问题:后向扫描逻辑是特定于轮次边界检测的,没有复制任何现有工具。
  • 接线正确:三个调用点(AppContainer.tsxuseResumeCommand.tsuseBranchCommand.ts)都使用 ?? 0 回退读取设置并传入。两个 hook 都将其添加到 useCallback 依赖数组中。

未发现正确性 bug、安全问题或结构性违规。

测试结果

单元测试(全部通过):

  • resumeHistoryUtils.test.ts: 24 / 24
  • useResumeCommand.test.ts (11) + useBranchCommand.test.ts (17): 28 / 28
  • npm run typecheck(全部 6 个包): exit 0 ✓
  • npm run lint: exit 0 ✓
  • npm run generate:settings-schema: 无 diff(schema 已更新——之前的 CI 阻塞已解决)✓

真实场景测试(tmux)

创建了一个 2 轮会话("say hello" → "now say goodbye"),然后用不同的 collapsePreviewCount 值恢复。

collapsePreviewCount=1(部分预览)

最后一轮可见("now say goodbye" + 响应),第一轮被折叠,摘要显示"2 messages hidden"。✓

collapsePreviewCount=0(默认,全部折叠)

所有历史折叠,无轮次预览,摘要显示"7 messages hidden"(包含意外交互测试中增加的额外轮次)。默认行为不变。✓

Qwen Code · qwen3.7-max

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Reflection

This is a well-executed, focused PR. The problem is real (all-or-nothing collapse makes it hard to orient yourself when resuming), the solution is minimal (one setting, one function change, three wiring updates), and the execution is clean.

My independent proposal would have been essentially the same: add a numeric setting that controls how many recent user turns to keep visible, scan backward from the end for MessageType.USER boundaries, and adjust the summary count to reflect only the actually-hidden items. The PR's implementation matches this exactly, with the nice addition of -1 as a "show all" escape hatch.

The tmux tests confirm both the new partial-preview behavior and the backward-compatible default collapse. All 52 unit tests pass, typecheck and lint are clean, and the settings schema has been regenerated (resolving the previous CI blocker).

The one non-blocking nit (no integer/minimum constraint on the number setting) was already flagged by the maintainer's earlier review and is cosmetic robustness only — out-of-range values produce harmless behavior.

Verdict: Approve. ✅

中文说明

反思

这是一个执行良好、范围集中的 PR。问题是真实的(全有或全无的折叠使得恢复时难以定位),解决方案是最小的(一个设置、一个函数变更、三个接线更新),执行干净。

我的独立方案本质上是相同的:添加一个数字设置来控制保留多少最近的用户轮次可见,从末尾向前扫描 MessageType.USER 边界,并调整摘要计数以仅反映实际隐藏的项目。PR 的实现完全匹配这一点,并额外添加了 -1 作为"显示全部"的逃生通道。

tmux 测试确认了新的部分预览行为和向后兼容的默认折叠。所有 52 个单元测试通过,typecheck 和 lint 干净,settings schema 已重新生成(解决了之前的 CI 阻塞)。

一个不阻塞的小问题(数字设置缺少 integer/minimum 约束)已在维护者之前的审查中标记,仅属于健壮性细节——越界值产生无害行为。

结论: 批准 ✅

Qwen Code · qwen3.7-max

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, looks ready to ship. ✅

@wenshao

wenshao commented Jun 29, 2026

Copy link
Copy Markdown
Collaborator

✅ Maintainer re-verification at HEAD c19c3c0 — real-TUI build test (tmux) + base A/B + mutation

Follow-up to my earlier review (which flagged the stale generated settings.schema.json): that blocker is now fixed by c19c3c0 chore: regenerate settings schema, and Test (ubuntu-latest) is green. I rebuilt the real qwen binary from the current head (npm ci && npm run build) and drove it in tmux end-to-end.

Bottom line: behavior matches the spec on every branch, the new logic is load-bearing (proven by a base A/B), the tests are non-vacuous, typecheck/CI are green — merge-ready. Two non-blocking nits below.

1. Real-TUI behavior matrix

Recorded a real 4-turn session with glm-5.2 (ALPHA111 → BRAVO222 → CHARLIE333 → DELTA4448 restored history items: 4 user + 4 assistant), then resumed it with qwen --resume <id> under each setting (collapseOnResume: true):

collapsePreviewCount Visible turns in the TUI Summary line
0 (default) (none) ● History collapsed: 8 messages hidden
1 DELTA444 ● History collapsed: 6 messages hidden
2 CHARLIE333, DELTA444 ● History collapsed: 4 messages hidden
-1 all 4 turns (no summary)
5 (> turns) all 4 turns (no summary)

The count invariant is exact: each turn = 2 items, so keeping N turns hides 8 − 2N, and the summary counts only the hidden items. Real count=2 capture:

  > Reply with exactly: CHARLIE333 and nothing else
  ✦ …CHARLIE333
  > Reply with exactly: DELTA444 and nothing else
  ✦ …DELTA444
  ● History collapsed: 4 messages hidden. Use /history expand-now to show.

2. Base A/B — the decisive proof the new code is load-bearing

Reverted only applyCollapsePolicyAndSummary to its pre-PR body (keeping the signature so tsc passes), rebuilt the CLI, and resumed the same session at count=2:

Binary count=2 result
PR c19c3c0 last 2 turns visible · 4 messages hidden
base (mutant) everything collapsed · 8 messages hidden — setting silently ignored ❌

So the preview behavior comes entirely from this PR; without it the setting is a no-op.

3. Both resume entry points exercised live

  • qwen --resume <id>AppContainer.tsx
  • /resume <id> slash command → useResumeCommand.ts ✅ (same count=2 → last 2 turns + 4 messages hidden)
  • /branch (useBranchCommand.ts) not driven live, but it is the identical 2-line wiring and its 17 unit tests pass.
  • /history expand-now after a partial collapse re-revealed all 4 turns and dropped the summary → canonical history is untouched, exactly as the PR claims ("only the suppressOnRestore display flag is varied").

4. Unit tests + non-vacuousness

  • 62 / 62 pass at head: resumeHistoryUtils.test.ts (24) + useResumeCommand.test.ts (11) + useBranchCommand.test.ts (17) + my own independent suite (10 — harder data: interleaved tool_group items, a leading non-user item, out-of-range counts).
  • Mutation: with the function reverted to base, 8 count-sensitive tests fail (3 from the PR's suite — partial preview / preview-covers-all / -1; 5 from mine), while the collapse-all and collapseOnResume:false tests stay green (correctly base-compatible). The new tests genuinely guard the change.
  • npm run typecheck (cli) clean on the merged head.

5. CI / merge state

Test (ubuntu-latest, Node 22.x) pass 21m32s (the leg that runs the schema check + unit tests). PR is MERGEABLE · CLEAN · APPROVED.

6. Non-blocking nits (cosmetic robustness — unchanged from my earlier note)

  1. collapsePreviewCount is type: "number" with no integer/min constraint. Verified live: 2.5 and -2 both silently collapse everything (8 messages hidden) instead of a partial preview — only -1 is the "show all" sentinel, and only positive integers yield a preview. Surprising but harmless (the default 0 path is byte-for-byte unchanged). Optional hardening: coerce to integer and clamp to >= -1.
  2. Summary placement: on a partial collapse the summary renders below the visible recent turns (…DELTA444 then 4 messages hidden), consistent with the existing collapse-all placement. Since the hidden items are the older ones, a top placement might read more naturally — purely a design call, not a correctness issue.

Verdict: verified merge-ready. Real-TUI behavior is correct on every branch, the new logic is load-bearing, tests are non-vacuous, and CI is green. The nits are optional polish, not blockers.

🇨🇳 中文版(点击展开)

✅ 维护者复验(HEAD c19c3c0)—— 真实 TUI 构建测试(tmux)+ 基线 A/B + 变异测试

接续我之前的评审(当时指出生成的 settings.schema.json 没重新生成):那个阻塞已由 c19c3c0 chore: regenerate settings schema 修复Test (ubuntu-latest) 现在是绿的。我从当前 head 重新构建了真实的 qwen 二进制npm ci && npm run build),并在 tmux 里端到端驱动验证。

结论:每个分支的行为都与规格一致;新逻辑确实承重(已用基线 A/B 证明);测试非空过;typecheck/CI 全绿 —— 可以合并。 下面两条是非阻塞的小问题。

1. 真实 TUI 行为矩阵

glm-5.2 录制了一个真实的 4 轮会话(ALPHA111 → BRAVO222 → CHARLIE333 → DELTA444,恢复后共 8 条历史项:4 条 user + 4 条 assistant),再用 qwen --resume <id> 在各设置下恢复(collapseOnResume: true):

collapsePreviewCount TUI 中可见的轮次 摘要行
0(默认) (无) ● History collapsed: 8 messages hidden
1 DELTA444 ● History collapsed: 6 messages hidden
2 CHARLIE333、DELTA444 ● History collapsed: 4 messages hidden
-1 全部 4 轮 (无摘要)
5(> 轮数) 全部 4 轮 (无摘要)

计数不变式精确成立:每轮 2 条,保留 N 轮即隐藏 8 − 2N,摘要统计被隐藏的条目。

2. 基线 A/B —— 证明新代码承重的关键证据

applyCollapsePolicyAndSummary 还原成 PR 前的函数体(保留签名让 tsc 通过),重建 CLI,再用同一个会话在 count=2 下恢复:

二进制 count=2 结果
PR c19c3c0 最后 2 轮可见 · 4 messages hidden
基线(变异体) 全部折叠 · 8 messages hidden,设置被静默忽略 ❌

即:preview 行为完全来自本 PR,没有它该设置就是个空操作。

3. 两个恢复入口都做了实测

  • qwen --resume <id>AppContainer.tsx
  • /resume <id> 斜杠命令 → useResumeCommand.ts ✅(同样 count=2 → 最后 2 轮 + 4 messages hidden
  • /branchuseBranchCommand.ts)未实测,但它是完全相同的两行接线,其 17 个单测通过。
  • 部分折叠后执行 /history expand-now 重新显示了全部 4 轮并去掉了摘要 → 规范历史未被改动,与 PR 声明一致("只改 suppressOnRestore 显示标志")。

4. 单元测试 + 非空过验证

  • head 上 62 / 62 通过resumeHistoryUtils.test.ts(24) + useResumeCommand.test.ts(11) + useBranchCommand.test.ts(17) + 我自己的独立套件(10 —— 用了更难的数据:穿插的 tool_group 项、起首的非 user 项、越界计数)。
  • 变异测试: 把函数还原成基线后,8 个与计数相关的测试失败(PR 套件里 3 个 —— 部分预览/预览覆盖全部/-1;我的 5 个),而 collapse-all 和 collapseOnResume:false 的测试仍绿(与基线兼容,正确)。新测试确实守住了这次改动。
  • npm run typecheck(cli)在合并后的 head 上干净。

5. CI / 合并状态

Test (ubuntu-latest, Node 22.x) 通过 21m32s(跑 schema 检查 + 单测的那条腿)。PR 为 MERGEABLE · CLEAN · APPROVED

6. 非阻塞小问题(健壮性打磨,与上次一致)

  1. collapsePreviewCounttype: "number",没有整数/最小值约束。 实测:2.5-2 都会静默折叠全部8 messages hidden),而不是部分预览 —— 只有 -1 是 "全显" 哨兵值,且只有正整数才给预览。意外但无害(默认 0 路径逐字节不变)。可选加固:强制取整并钳到 >= -1
  2. 摘要位置: 部分折叠时摘要渲染在可见的近期轮次下方…DELTA444 之后才是 4 messages hidden),与既有的 collapse-all 摆放一致。但被隐藏的是更早的内容,放到顶部可能更直观 —— 纯设计取舍,非正确性问题。

结论:复验通过,可合并。 每个分支的真实 TUI 行为都正确,新逻辑承重,测试非空过,CI 全绿;两条小问题属可选打磨,不阻塞合并。

Verified locally on macOS by building the real binary from c19c3c0 and driving it in tmux (recorded a live 4-turn session, resumed under each setting, base-A/B via source revert + CLI rebuild, vitest mutation). Not a substitute for the author's own platform testing.

@DragonnZhang DragonnZhang left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Automated code review — no high-confidence critical findings in the changed code at this commit.

Reviewed applyCollapsePolicyAndSummary with the new collapsePreviewCount parameter. The boundary logic correctly handles: 0 (collapse all), -1 (show all, return early), N > available user turns (boundary=0, show all without summary), N ≤ available user turns (hide items before boundary). The summary count reflects boundary (hidden items only), not total items. Tests cover all cases including empty history. No critical issues found.


Generated by Claude Code

@wenshao

wenshao commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Maintainer note: current head c19c3c0 is APPROVED + CLEAN, CI is green, and the stale generated settings.schema.json blocker is fixed. I think this is mergeable as-is.

The remaining unresolved review threads are suggestions / nice-to-haves rather than blockers. The two follow-ups I would keep on the radar are:

  1. Tighten ui.history.collapsePreviewCount schema to an integer with minimum: -1, so fractional or other negative values cannot produce surprising behavior.
  2. Add a couple of boundary tests, especially collapsePreviewCount=1 and preview count greater than the number of user turns.

These can be handled either as a tiny pre-merge polish commit or a post-merge follow-up. They are not blocking from my side.

@wenshao
wenshao added this pull request to the merge queue Jun 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jun 30, 2026
@wenshao
wenshao added this pull request to the merge queue Jun 30, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to no response for status checks Jun 30, 2026
@wenshao
wenshao added this pull request to the merge queue Jun 30, 2026
Merged via the queue into QwenLM:main with commit f3694dd Jun 30, 2026
48 checks passed
chiga0 pushed a commit to chiga0/qwen-code that referenced this pull request Jun 30, 2026
Resolve conflict in docs/users/configuration/settings.md: keep main's
new `ui.history.collapsePreviewCount` row (QwenLM#5848) alongside this
branch's updated `ui.compactMode` description. The incoming side kept
the stale "Toggle with Ctrl+O during a session" wording, which is wrong
now that this branch retired compactMode in the terminal UI (Ctrl+O
opens the full-detail transcript, it no longer toggles a mode), so the
retired-in-TUI description is preserved.

settingsSchema.ts, settings.schema.json, AppContainer.tsx and
resumeHistoryUtils.ts auto-merged cleanly; regenerating the schema
produced no diff, confirming the auto-merge was correct.

Generated with AI

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(ui): add ui.history.collapsePreviewCount to show last N messages when resuming collapsed sessions

5 participants