Skip to content

fix: 优化 init 部分失败的汇总展示#145

Merged
benym merged 3 commits into
rpamis:masterfrom
ddddddddwp:codex/issue-128-opencode-summary
Jun 27, 2026
Merged

fix: 优化 init 部分失败的汇总展示#145
benym merged 3 commits into
rpamis:masterfrom
ddddddddwp:codex/issue-128-opencode-summary

Conversation

@ddddddddwp

@ddddddddwp ddddddddwp commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

变更内容

  • 调整 comet init 最终汇总的分类逻辑:只要某个平台存在失败组件,就不再把该平台列入 Installed
  • Failed 汇总中显示失败组件,例如 OpenCode (OpenSpec failed)
  • 增加 OpenCode 部分失败场景的回归测试,覆盖 OpenSpec 初始化失败但 Comet skills 已复制成功的输出。

背景

此前最终汇总按组件状态分别归类,导致同一平台在部分成功、部分失败时可能同时出现在 InstalledFailed。新的输出保留组件级过程日志,同时让最终汇总只突出失败部分,避免误导用户认为平台整体安装成功。

验证

  • npx vitest run test/ts/init-e2e.test.ts test/ts/init.test.ts
  • pnpm run build
  • git diff --check HEAD

Summary by CodeRabbit

  • Bug Fixes
    • Improved comet init setup summary: platforms with failed components are excluded from Installed and failures are explicitly listed under Failed with per-component details.
    • Added a new localized translation key for the failure status message (failed) to ensure consistent messaging across supported languages.
    • Stabilized end-to-end test output by mocking version/update info and capturing combined console output, improving validation reliability.

@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 488b5e84-8107-4d0c-93bc-27fce9acca3f

📥 Commits

Reviewing files that changed from the base of the PR and between 6146e82 and 6f8b06f.

📒 Files selected for processing (3)
  • CHANGELOG.md
  • src/commands/init.ts
  • test/ts/init-e2e.test.ts
✅ Files skipped from review due to trivial changes (1)
  • CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/commands/init.ts
  • test/ts/init-e2e.test.ts

📝 Walkthrough

Walkthrough

The 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.

Changes

Init failure summary

Layer / File(s) Summary
Translation key and labels
src/commands/i18n.ts
Adds failedStatus to the translation key union and to the English and Chinese translation maps.
Failed summary formatting
src/commands/init.ts
Derives status groups from shared helpers and prints per-platform failure details instead of a joined list of failed platform names.
E2E failure assertion
test/ts/init-e2e.test.ts, CHANGELOG.md
Mocks version output, captures console text, asserts the OpenCode failure summary, and adds the changelog note for partial initialization failures.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • rpamis/comet#109: Also changes comet init summary and i18n handling for setup output.

Poem

A bunny hopped through init with glee,
and found a failed-status label in the tree.
When OpenCode tripped, the summary stayed fair,
with failure details printed right there.
🐇

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly matches the PR’s main change: improving how partial init failures are summarized.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ddddddddwp ddddddddwp marked this pull request as ready for review June 26, 2026 09:57
@greptile-apps

greptile-apps Bot commented Jun 26, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes the comet init summary display so that platforms with any failed component are excluded from Installed and instead grouped under Failed with per-component details (e.g., OpenCode (OpenSpec failed)). Previously, a partially-installed platform could simultaneously appear in both sections, misleading users into thinking installation succeeded.

  • init.ts: Refactors displaySummary — replaced four repeated OR/AND per-field filter expressions with a shared componentStatuses table and three predicates (hasFailure, hasInstall, failedDetails); classification is now mutually exclusive.
  • i18n.ts: Adds a new failedStatus translation key ('failed' / '失败') used to format per-component failure labels in the summary.
  • init-e2e.test.ts: Adds a regression test for the partial-OpenCode-failure scenario, mocks version.js to suppress live update checks, and introduces captureTextOutput to capture both console.log and console.error output.

Confidence Score: 5/5

Safe 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 displaySummary refactor replaces duplicated per-field OR/AND filters with a shared table and three clean predicates. A platform with any failed component is now excluded from Installed and placed exclusively in Failed with component-level detail. No data flows or external contracts are affected; the fix is purely presentational. The new E2E test correctly validates that OpenCode is absent from Installed and present exactly once in Failed when OpenSpec initialization throws. Both language translations are present and consistent.

No files require special attention. The only open item is a missing ### Tests CHANGELOG bullet, which is a process note rather than a code concern.

Important Files Changed

Filename Overview
src/commands/init.ts Refactors displaySummary with a shared component-status table; the three result buckets are now mutually exclusive and correctly handle partial-failure platforms.
src/commands/i18n.ts Adds failedStatus translation key for English ('failed') and Chinese ('失败'); no other translation keys are affected.
test/ts/init-e2e.test.ts Adds regression test for partial OpenCode failure; mocks version info to stabilize output. captureTextOutput appends all console.error lines after all console.log lines rather than preserving real interleave order, but this is harmless for presence-based assertions.
CHANGELOG.md Adds a Fixed bullet for the summary reclassification. Per AGENTS.md, a ### Tests section documenting the new regression coverage is missing.

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, ..."]
Loading
%%{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, ..."]
Loading

Reviews (3): Last reviewed commit: "chore: 重新触发 CI" | Re-trigger Greptile

Comment thread src/commands/init.ts

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (1)
src/commands/init.ts (1)

264-269: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Derive skipped from componentStatuses too.

failed and installed now share the component list, but skipped still 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

📥 Commits

Reviewing files that changed from the base of the PR and between 7a500dc and 6146e82.

📒 Files selected for processing (3)
  • src/commands/i18n.ts
  • src/commands/init.ts
  • test/ts/init-e2e.test.ts

Comment thread test/ts/init-e2e.test.ts
@benym

benym commented Jun 27, 2026

Copy link
Copy Markdown
Contributor

LGTM

@benym benym merged commit bc6219d into rpamis:master Jun 27, 2026
14 checks passed
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.

2 participants