fix: 优化 init 部分失败的汇总展示#145
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe init command now localizes a failed-status label, recalculates summary groups with shared status helpers, prints per-platform failure details, and adds E2E coverage and changelog text for partial failure output. ChangesInit failure summary
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Greptile SummaryThis PR fixes the
Confidence Score: 5/5Safe to merge — the classification change is logically correct, the three result buckets are mutually exclusive, and the regression test exercises the partial-failure path end-to-end. The No files require special attention. The only open item is a missing Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[PlatformResult list] --> B{hasFailure?\nany component === 'failed'}
B -- Yes --> C[failed bucket]
B -- No --> D{hasInstall?\nany component === 'installed'}
D -- Yes --> E[installed bucket]
D -- No --> F[skipped bucket\nall components === 'skipped']
C --> G["Print: Failed:\n PlatformName (Component failed, ...)"]
E --> H["Print: Installed:\n PlatformName -> dir/skills/"]
F --> I["Print: Skipped: PlatformName, ..."]
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
A[PlatformResult list] --> B{hasFailure?\nany component === 'failed'}
B -- Yes --> C[failed bucket]
B -- No --> D{hasInstall?\nany component === 'installed'}
D -- Yes --> E[installed bucket]
D -- No --> F[skipped bucket\nall components === 'skipped']
C --> G["Print: Failed:\n PlatformName (Component failed, ...)"]
E --> H["Print: Installed:\n PlatformName -> dir/skills/"]
F --> I["Print: Skipped: PlatformName, ..."]
Reviews (3): Last reviewed commit: "chore: 重新触发 CI" | Re-trigger Greptile |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/commands/init.ts (1)
264-269: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive
skippedfromcomponentStatusestoo.
failedandinstallednow share the component list, butskippedstill hard-codes the four fields. The next component rename/addition can desynchronize the summary buckets again.Suggested change
- const skipped = results.filter( - (r) => - r.openspec === 'skipped' && - r.superpowers === 'skipped' && - r.comet === 'skipped' && - r.codegraph === 'skipped', - ); + const skipped = results.filter((r) => + componentStatuses.every(([key]) => r[key] === 'skipped'), + );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/commands/init.ts` around lines 264 - 269, The skipped summary in the init flow is still hard-coding individual component fields, so it can drift when components change. Update the `results.filter(...)` logic in `src/commands/init.ts` to derive `skipped` from the same shared `componentStatuses` structure used for `failed` and `installed`, so component additions/renames stay in sync across all summary buckets.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/ts/init-e2e.test.ts`:
- Around line 364-366: The failure summary assertion in init-e2e.test.ts is too
weak because it only checks presence of “OpenCode (OpenSpec failed)”. Update the
expectations around the summary output in the affected test so it verifies the
failed summary appears exactly once, using the existing output variable and the
same OpenCode/OpenSpec failed label to make duplicates fail the test.
---
Nitpick comments:
In `@src/commands/init.ts`:
- Around line 264-269: The skipped summary in the init flow is still hard-coding
individual component fields, so it can drift when components change. Update the
`results.filter(...)` logic in `src/commands/init.ts` to derive `skipped` from
the same shared `componentStatuses` structure used for `failed` and `installed`,
so component additions/renames stay in sync across all summary buckets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f23ffa24-db29-4446-9cdf-8774d72a8a92
📒 Files selected for processing (3)
src/commands/i18n.tssrc/commands/init.tstest/ts/init-e2e.test.ts
|
LGTM |
变更内容
comet init最终汇总的分类逻辑:只要某个平台存在失败组件,就不再把该平台列入Installed。Failed汇总中显示失败组件,例如OpenCode (OpenSpec failed)。背景
此前最终汇总按组件状态分别归类,导致同一平台在部分成功、部分失败时可能同时出现在
Installed和Failed。新的输出保留组件级过程日志,同时让最终汇总只突出失败部分,避免误导用户认为平台整体安装成功。验证
npx vitest run test/ts/init-e2e.test.ts test/ts/init.test.tspnpm run buildgit diff --check HEADSummary by CodeRabbit
comet initsetup summary: platforms with failed components are excluded from Installed and failures are explicitly listed under Failed with per-component details.failed) to ensure consistent messaging across supported languages.