refactor(app): remove skill-card shortcuts and home-override pipeline (#603 PR2/3)#657
Conversation
The three skill cards on the session-less home (Process docs /
Analyze data / Start writing) and their entire submit-pipeline
plumbing are removed. The cards were UI shortcuts that pre-filled
a slash command into the composer via an outgoingTextOverride field
on FollowupDraft and a !homeSkill branch in createPromptSubmit.
Users keep the underlying capability: the backend slash commands
(/document-processing, /data-analysis, /writing-assistant) are
untouched and can be typed directly in the composer. Removing the
plumbing avoids leaving a dead override path in submit.ts that PR3
would otherwise need to step around.
Delete (4): components/session/pawwork-skill-meta.{ts,test.ts},
components/prompt-input/home-override.{ts,test.ts}.
Modify (16): prompt-input.tsx, prompt-input/{placeholder,readiness,
submit}.{ts,test.ts}, session/session-new-view.tsx, i18n/{zh,en,
parity.test}.ts, pages/session.tsx, pages/session/{session-main-view,
session-composer-region,composer/session-composer-region}.tsx,
pages/session/{session-action-readiness,use-session-followups}.test.ts.
Net: +10 / -290; 18 i18n keys gone (9 zh, 9 en); FollowupDraft
loses outgoingTextOverride; createPromptSubmit loses homeSkill
derivation + 3 branches; placeholder.ts loses SKILL_PLACEHOLDER_KEY;
readiness.ts loses selectedSkill clause; session page chain (4 files)
loses selectedSkill prop drilling.
session-new-view.tsx keeps its current h1 + composer shape; PR3
rewrites it as the W5 hero (h1 + meta-path button + composer).
The homeMode prop in prompt-input.tsx is kept (still gates
isNewSession at L441 and the chip-render block at L673 that PR3
will edit).
Slice 2 of 3 for #603. See plan
docs/superpowers/plans/2026-05-15-i603-w5-home.md Phase 2 and
issue comment 4460021650 for the full design + 3-slice inventory.
Refs #603.
📝 WalkthroughWalkthroughThis PR removes the Pawwork skill selection feature and its metadata, simplifies prompt placeholders to suggestion-based logic, removes home-skill overrides and outgoing-text overrides from submit flow, narrows composer/component contracts by removing ChangesSkill selection feature removal
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Suggested priority: P2 (includes user-path files (packages/app/src/components/prompt-input.tsx, packages/app/src/components/prompt-input/home-override.test.ts, packages/app/src/components/prompt-input/home-override.ts, packages/app/src/components/prompt-input/placeholder.test.ts, packages/app/src/components/prompt-input/placeholder.ts, packages/app/src/components/prompt-input/readiness.test.ts, packages/app/src/components/prompt-input/readiness.ts, packages/app/src/components/prompt-input/submit.ts, packages/app/src/components/session/pawwork-skill-meta.test.ts, packages/app/src/components/session/pawwork-skill-meta.ts, packages/app/src/components/session/session-new-view.tsx, packages/app/src/i18n/en.ts, packages/app/src/i18n/parity.test.ts, packages/app/src/i18n/zh.ts, packages/app/src/pages/session.tsx, packages/app/src/pages/session/composer/session-composer-region.tsx, packages/app/src/pages/session/session-action-readiness.test.ts, packages/app/src/pages/session/session-composer-region.tsx, packages/app/src/pages/session/session-main-view.tsx, packages/app/src/pages/session/use-session-followups.test.ts)).
P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.
There was a problem hiding this comment.
Code Review
This pull request removes the 'Pawwork Skills' feature from the application. The changes include deleting the skill metadata and helper functions, removing skill-related properties from the prompt input and session components, and cleaning up associated unit tests and internationalization strings. I have no feedback to provide as there were no review comments to assess.
The starter-cards smoke assertion was the only e2e reference to the three skill cards (Process docs / Analyze data / Start writing) and broke on CI after the cards were removed in the same PR. Flip the assertion direction so it now guards card absence, and extend the single-row composer test to cover the send-button disabled / enabled / disabled cycle around the blank-prompt readiness rule that submit.ts simplified. Add a new smoke spec submitting an unregistered slash-prefixed prompt (/pr2skillcheck) to lock the slash-bypass fall-through path that the PR simplified — removing the !homeSkill guard must still let a /-prefixed prompt with no matching command fall through to the standard prompt submit pipeline. Targeted run: 5 home smoke tests passed locally (chromium, 12.8s).
Perf delta summaryComparator: pass
|
The expected @smoke inventory in opencode's e2e-smoke-tagging.test.ts is asserted exactly equal to the discovered list of @smoke titles. The previous commit renamed home.spec.ts's starter-cards test and added a slash-prefixed prompt test, so the inventory drifted and unit-opencode CI failed on the equality assertion. Update the inventory to match: - replace "home renders the hero composer and starter cards" with "home renders hero composer without skill-card shortcuts" - add "home composer submits a slash-prefixed prompt via the fallback path" Targeted run: 2 inventory tests passed locally.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/app/e2e/app/home.spec.ts (1)
61-61: ⚡ Quick winUse canonical workspace route assertion for session navigation.
This should assert the resolved/canonical workspace slug route instead of only checking
"/session/", so Windows/workspace normalization differences are covered.As per coding guidelines, "When validating routing, assert against canonical or resolved workspace slugs using shared helpers from
../actionsto account for Windows canonicalization".🤖 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 `@packages/app/e2e/app/home.spec.ts` at line 61, Replace the loose "/session/" URL assertion with a check against the canonical/resolved workspace route from the shared actions helper: import and call the helper (e.g. getCanonicalWorkspaceSlug or getResolvedWorkspaceRoute from "../actions") and then assert the polled page.url() contains `/session/${canonicalSlug}` (or the resolved route string) with the same timeout; update the assertion that currently references page.url() to use the helper-provided slug/route so Windows/workspace normalization is covered.
🤖 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.
Nitpick comments:
In `@packages/app/e2e/app/home.spec.ts`:
- Line 61: Replace the loose "/session/" URL assertion with a check against the
canonical/resolved workspace route from the shared actions helper: import and
call the helper (e.g. getCanonicalWorkspaceSlug or getResolvedWorkspaceRoute
from "../actions") and then assert the polled page.url() contains
`/session/${canonicalSlug}` (or the resolved route string) with the same
timeout; update the assertion that currently references page.url() to use the
helper-provided slug/route so Windows/workspace normalization is covered.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d046d360-b11c-467e-9522-c2ad47bab5ef
📒 Files selected for processing (2)
packages/app/e2e/app/home.spec.tspackages/opencode/test/config/e2e-smoke-tagging.test.ts
|
Re CodeRabbit nitpick on `packages/app/e2e/home.spec.ts:61` (canonical workspace slug assertion): Declining for this PR. Four reasons:
If a reviewer wants this tightened repo-wide, I'll do it in a focused PR. |
## Summary
Refresh the home `<h1>` to the W5 welcome wording locked by the design preview ("今天我们做点什么?" / "What should we work on?") and tune the heading typography toward `--type-display`: medium weight, 28/130, and the documented zh-CJK letter-spacing rule (CJK 0, en tightest). Widen the heading-to-composer gap from `mt-12` to `mt-20` so the hero block breathes. Production layout, `WorkspaceChip` inside the composer, and every other home surface stay untouched.
## Why
PR1 (#651) cleared the dead home page and PR2 (#657) stripped the skill-card pipeline. PR3 originally planned to rewrite `NewSessionView` as a W5 hero (meta-path button + branch chip + extracted `WorkspacePopover`). The maintainer manual-tested the experiment in Electron and decided the cwd signal reads cleaner where it already is (inside the composer), and that the home page should stay minimal. Rolled back to the smallest change that delivers the welcome copy + typography refresh on the existing layout.
## Related Issue
Refs #603. Slice 3 of 3.
## Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
## Review Focus
- `session-new-view.tsx` — `--type-display` is rendered with arbitrary Tailwind values (`text-[28px] font-medium leading-[1.3]` + `tracking-[var(--letter-spacing-tightest|normal)]`) because there is no `text-display` utility class yet. Introducing one would be a cross-package utility addition (would touch `packages/ui` and the error-page heading); kept out of scope and noted as a follow-up candidate.
- i18n: `home.hero.title` replaces `session.new.title`. Both `parity.test.ts` and the `home.spec.ts` heading assertion are updated. `session.new.title` is removed in both locales — grep confirms no other consumer.
- The opencode `@smoke` inventory in `packages/opencode/test/config/e2e-smoke-tagging.test.ts` is updated alphabetically to match the new home smoke title.
## Risk Notes
None. No behavioral or platform-impacting change. No new schema, no migration, no IPC surface touched.
## How To Verify
```text
packages/app typecheck (tsgo -b): clean
packages/opencode typecheck (tsgo --noEmit): clean
packages/app unit tests (bun test src/): 1103 pass / 0 fail / 2649 expect()
packages/app E2E home.spec.ts (chromium): 6 pass / 10.0s
packages/opencode e2e-smoke-tagging unit: 2 pass
Electron `bun run dev:desktop` manual check: heading copy + medium-weight display tier + heading↔composer gap visible; WorkspaceChip remains inside composer; zh + en letter-spacing rules both render
Crosscheck (Claude Opus + Codex): 0 P0 / 0 P1; one P2 (`tracking-[var(--letter-spacing-tightest)]` token swap) applied in-PR; other P2/P3 deferred (new utility class out of scope; pre-existing parity-test observation)
```
## Screenshots or Recordings
Manual screenshot to be attached after PR open — the visible change is the heading copy ("今天我们做点什么?" / "What should we work on?") plus the slightly heavier medium-weight 28/130 display heading and the wider heading-to-composer gap. The composer, workspace chip, and overall layout are byte-for-byte unchanged from production.
## Checklist
- [x] Human review status is stated above as pending, approved, or not required
- [x] I linked the related issue, or stated why there is no issue
- [x] This PR has type, primary area, and priority labels, or I requested maintainer labeling
- [x] I described the review focus and any meaningful risks
- [x] I listed the relevant verification steps and the key result for each
- [x] I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope
- [x] I manually checked visible UI or copy changes when needed, with screenshots or recordings
- [x] I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes
- [x] I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant
- [x] I reviewed the final diff for unrelated changes and suspicious dependency changes
- [x] I am targeting `dev`, and my PR title and commit messages use Conventional Commits in English
Summary
Remove the three skill cards (Process docs / Analyze data / Start writing) on the session-less home, and strip the entire composer-submit override path that backed them.
Net: 4 files deleted, 17 modified, +51 / -304; 18 i18n keys removed (9 zh, 9 en).
Why
The cards were UI shortcuts that pre-filled a slash command into the composer through an
outgoingTextOverridefield onFollowupDraftand ahomeSkillderivation + 3 branches increatePromptSubmit. PR3 rewritessession-new-view.tsxas the W5 hero (h1 + meta-path button + composer); leaving the override plumbing in place would force PR3 to step around dead code insubmit.ts.Users keep the underlying capability: the backend slash commands
/document-processing,/data-analysis,/writing-assistantare untouched and can be typed directly in the composer (the productivity skill discovery test inpackages/opencode/test/skill/skill.test.ts:414-433still passes since this PR does not touch the backend).session-new-view.tsxkeeps its current h1 + composer shape in this slice; PR3 rewrites it. ThehomeModeprop inprompt-input.tsxstays (still gatesisNewSessionat L441 and the chip-render block at L673 that PR3 will edit).Related Issue
Refs #603. Slice 2 of 3. Full design + 3-slice inventory in issue comment 4460021650. Plan:
docs/superpowers/plans/2026-05-15-i603-w5-home.mdPhase 2.Human Review Status
Pending. A human should make the final merge decision after reviewing the final diff and verification evidence.
Review Focus
submit.tsbehavioral deltas — four branches that depended onhomeSkillare simplified. The empty-text guard now allows submit only ifactionReady+canSubmitPromptagree (no behavioral change since cards were the only producer of empty-text-with-skill submissions). The!homeSkillhistory skip is removed (all submits land in history; matches what direct slash-command typing already did).followupCommandTextnow always returnsdraftText(draft.prompt). Thetext.startsWith("/")slash bypass is unconditional; if the slash command name is not registered insync.data.command, the path falls through to the standard prompt pipeline as before — locked by the newsubmits a slash-prefixed prompt via the fallback pathE2E spec.session-new-view.tsxsimplification — old code passed a realsetModefromcreateSignalto the composer; new code passes() => {}because nothing in the new-view path consumes mode after card removal (prompt-inputowns the authoritative mode store internally). PR3 rewrites this view, so the noop is short-lived.use-session-followups.test.ts:60-64— mock now mirrors the newfollowupCommandTextsource exactly.Risk Notes
None. Backend untouched; no migrations; no platform-specific behavior touched.
How To Verify
The P3 nit is the
() => {}noop insession-new-view.tsx:18— accepted in this PR; PR3 rewrites the view.Screenshots or Recordings
The visible UI change is the removal of the three skill-card buttons under the home composer. The new
home renders hero composer without skill-card shortcutssmoke spec asserts the cards are absent (toHaveCount(0)) and the rest of the home surface (h1 / composer / prompt / workspace chip / utility panel) renders as before. The Playwrighthome.spec.tsartifact in the e2e-artifacts upload captures the post-removal screenshot if a static image is needed.Checklist
dev, and my PR title and commit messages use Conventional Commits in EnglishSummary by CodeRabbit