Uh oh!
There was an error while loading. Please reload this page.
test(desktop): story-cover the change panel's failure and edge states - #4169
Conversation
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for exercising the real Workbar → Changes → SessionReviewPanel path and for avoiding fabricated error UI.
I reviewed exact head 7c3649731bedd536370bb7a798ba3afd079ac7c2. The exact-head checks are green and I found no P0 or P1 issues. The PR body, however, contains no screenshots or recordings of the five new visual states. Please attach the empty and failure states plus expanded truncated and edge-content rows at normal and narrow Workbar widths, including light and dark themes.
I found two non-blocking P2 coverage issues:
ChangesTruncatedcombines two independent authorities into a scenario production would not create. A single 620-line file triggers the panel-level 500-lineboundedDiff, but it does not set the snapshot-leveltruncatedflag; that flag comes from the source file-count or total-diff limits. Please separate per-file truncation from source snapshot truncation, or make the fixture satisfy the real source limit.ChangesEdgeContentclaims binary, rename, deletion, no-newline-at-EOF, and a long minified line, but its play only expands and verifies the binary row. Please exercise the other four bodies, especially long-line overflow and the EOF marker, or narrow the coverage claim.
For manual visual review, please show that long paths, statistics, and chevrons remain visible at narrow width, and that all five edge rows use the existing Astryx and diff-renderer language without overflow.
These are test-modeling issues rather than production UI defects, so P2 is appropriate and they do not invalidate the overall direction.
Review analysis was assisted by Codex and an independent @reviewer agent. Astro-Han verified the exact head, real component path, truncation authorities, Storybook evidence, CI, and severity judgment, and owns this review.
中文对照
谢谢你通过真实的 Workbar → Changes → SessionReviewPanel 路径覆盖状态,也没有伪造错误 UI。
我审查了精确 head 7c3649731bedd536370bb7a798ba3afd079ac7c2。exact-head checks 已通过,没有 P0/P1。但 PR 正文没有五个视觉状态的截图或录屏。请补充空状态、失败状态,以及展开后的 truncated 和 edge-content 行,包括正常与窄 Workbar 宽度、浅色和深色主题。
有两个不阻塞的 P2 覆盖问题:
ChangesTruncated把两个独立 authority 组合成了生产不会生成的场景。单个 620 行文件只会触发 panel 的 500 行boundedDiff;snapshot-leveltruncated来自 source 文件数量或总 diff 上限。请拆开 per-file truncation 和 source snapshot truncation,或者让 fixture 真正满足 source limit。ChangesEdgeContent声称覆盖 binary、rename、deletion、EOF 无换行和超长 minified line,但 play 实际只展开并验证 binary。请验证其余四种 body,尤其是长行溢出和 EOF marker;否则应收窄覆盖声明。
人工视觉验收还需要确认:窄宽度下长路径、统计数字和 chevron 保持可见,五种 edge row 都继续使用既有 Astryx 与 diff renderer 语言且没有溢出。
这些是测试建模问题,不是生产 UI 缺陷,因此定为 P2,不否定整体方向。
本次审查分析由 Codex 和独立的 @reviewer 子代理协助;Astro-Han 核验了精确 head、真实组件路径、截断 authority、Storybook 证据、CI 和问题分级,并对本次 Review 负责。
7c36497 to
9d3aec8Compareliuxiaocs7
commented
Aug 29, 2026
Thanks for the review. Both P2s are addressed in the pushed revision (force-updated to
Smoke re-run: 202 stories green, per-story AX-tree audit clean. On visual evidence: I'm working in a headless/CI environment and can't attach screenshots to the PR directly. The states render deterministically in |
liuxiaocs7
commented
Aug 29, 2026
Added a Visual evidence section to the PR description (screenshots on the fork's release assets): the empty state, the load-failure banner, source-level truncation, the per-file line cap (expanded), and the edge-content rows (expanded) — normal/light and narrow/dark. |
9d3aec8 to
13a3e03CompareFourth surface under apache#3944 (one surface per PR): extend the 变更 panel (SessionReviewPanel, via Product/Session Workbar) with the failure and edge states that never show up in normal use. All drive the real panel through the fake review service — no fabricated UI. - ChangesEmpty — branch matches base: the panel's own EmptyState. - ChangesLoadFailed — `review.read` rejects: an error Banner with 重试. - ChangesSourceNotGit — a source that cannot be read (not a git repo) is a failure Banner, not an absence; the other read reasons share this branch. - ChangesTruncated — snapshot-level source truncation (`truncated`): a large changeset whose file list the source capped → the 变化过多 banner. - ChangesFileLineCap — the per-file 500-line `boundedDiff` cap on one long file (snapshot NOT truncated) → the "另有 N 行未显示" note. Separated from source truncation, which is an independent authority (review feedback). - ChangesEdgeContent — binary, rename, deletion, no-newline-at-EOF, and a very long minified line. The play expands each row and asserts its own diff body renders, scoped per file (review feedback). DiffCodePreview is a pure unified-diff string renderer with no edge-state props; the failure states live in this wrapper, which is where they are exercised here. Refs apache#3944, apache#3893 Generated-by: Claude Code
13a3e03 to
9bff73cCompare
Astro-Han
left a comment
There was a problem hiding this comment.
Thanks for separating the two truncation authorities and exercising every claimed edge-diff body. Reviewed at exact head 9bff73c4e2b9081779e065e4f77b79cd1fa36761: the previous P2 findings are closed, the stories continue through the real Workbar → Changes → SessionReviewPanel path, and exact-head CI is green. The standard visual states were manually accepted; a full theme/viewport/state screenshot matrix is not required. The small fixture-stat mismatch is test-only P3 cleanup and does not undermine the truncation or rendering assertions. I found no issue that should delay this story coverage.
Review analysis was assisted by Codex and independent reviewer agents. I verified the exact head, production component path, prior findings, CI, fixture impact, and manual visual decision, and I own this review.
中文对照
感谢你拆开两个 truncation authority,并逐一验证声明覆盖的特殊 diff body。我审查了精确 head 9bff73c4e2b9081779e065e4f77b79cd1fa36761:之前两条 P2 已闭合,Story 仍通过真实的 Workbar → Changes → SessionReviewPanel 路径,exact-head CI 已通过。标准视觉状态已经人工确认,不需要完整的主题/宽度/状态截图矩阵。fixture 统计的小偏差只是测试侧 P3 清理,不影响截断和渲染断言。没有发现需要延迟这项 Story 覆盖的问题。
本次分析由 Codex 和独立 reviewer 子代理协助;我核验了精确 head、生产组件路径、旧问题、CI、fixture 影响和人工视觉结论,并对本次 Review 负责。
Uh oh!
There was an error while loading. Please reload this page.
test(desktop): story-cover the change panel's failure and edge states
Fourth surface under #3944 (one surface per PR): extend the 变更 panel
(SessionReviewPanel, via Product/Session Workbar) with the failure and edge
states that never show up in normal use. All drive the real panel through the
fake review service — no fabricated UI.
review.readrejects: an error Banner with 重试.failure Banner, not an absence; the other read reasons share this branch.
truncated): a largechangeset whose file list the source capped → the 变化过多 banner.
boundedDiffcap on one long file(snapshot NOT truncated) → the "另有 N 行未显示" note. Separated from source
truncation, which is an independent authority (review feedback).
long minified line. The play expands each row and asserts its own diff body
renders, scoped per file (review feedback).
DiffCodePreview is a pure unified-diff string renderer with no edge-state props;
the failure states live in this wrapper, which is where they are exercised here.
Refs #3944, #3893
Generative tooling
Claude Code contributed substantially here — it authored these Storybook stories. The human contributor of record reviewed and submitted the change, and each commit carries a
Generated-by: Claude Codetrailer per CONTRIBUTING.md.Visual evidence
Storybook stories captured with Playwright (hosted on the fork's release assets).
changes-empty— normal / light · narrow / darkchanges-load-failed— normal / light · narrow / darkchanges-truncated— normal / light · narrow / darkchanges-file-line-cap— normal / light · narrow / darkchanges-edge-content— normal / light · narrow / dark