fix(app): restore home model selector hit area for v0.2.9#197
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (12)
🧰 Additional context used📓 Path-based instructions (2)packages/app/**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Files:
packages/app/e2e/**/*.spec.ts📄 CodeRabbit inference engine (packages/app/e2e/AGENTS.md)
Files:
🧠 Learnings (15)📓 Common learnings📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-22T08:49:44.563ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-20T14:36:04.099ZApplied to files:
📚 Learning: 2026-04-23T15:25:27.182ZApplied to files:
📚 Learning: 2026-04-23T07:23:23.849ZApplied to files:
🔇 Additional comments (2)
📝 WalkthroughWalkthroughAdds an E2E Changes
Sequence Diagram(s)(omitted — changes are focused on a test helper and a UI layout refactor without a multi-component sequential interaction requiring visualization) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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)
Comment |
There was a problem hiding this comment.
Code Review
This pull request updates the version of several packages to 0.2.9 and includes corresponding updates to the lockfile. It also adds a new E2E test to ensure the home model selector opens without footer overlap, supported by a new hit-testing utility. In the PromptInput component, the footer layout was refactored to handle different display modes. Feedback was provided regarding significant code duplication introduced in this refactor, suggesting a unified container approach to improve maintainability.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/app/e2e/prompt/prompt-footer-focus.spec.ts`:
- Around line 108-110: The hit-test call runs before the trigger is guaranteed
visible, causing flaky false-intercepts; before calling hitTest on the locator
named trigger (created from promptModelSelector and data-action="prompt-model"),
wait for the element to be visible using the Playwright test helpers (e.g.
expect(trigger).toBeVisible() or expect.poll(...) as appropriate) so the
layout/paint is settled, then call hitTest and proceed based on its result.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4f126c2c-0c36-4546-8a1c-c2c190d5821b
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
packages/app/e2e/prompt/prompt-footer-focus.spec.tspackages/app/package.jsonpackages/app/src/components/prompt-input.tsxpackages/desktop-electron/package.jsonpackages/opencode/package.jsonpackages/util/package.json
📜 Review details
🧰 Additional context used
📓 Path-based instructions (2)
packages/app/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (packages/app/AGENTS.md)
Always prefer
createStoreover multiplecreateSignalcalls in SolidJS
Files:
packages/app/src/components/prompt-input.tsxpackages/app/e2e/prompt/prompt-footer-focus.spec.ts
packages/app/e2e/**/*.spec.ts
📄 CodeRabbit inference engine (packages/app/e2e/AGENTS.md)
packages/app/e2e/**/*.spec.ts: Import test utilities from../fixturesinstead of@playwright/test
Test files should be named with the patternfeature-name.spec.ts
Use lowercase, descriptive test names (e.g., 'sidebar can be toggled')
Use camelCase for variable names in tests
Use SCREAMING_SNAKE_CASE for constants in tests
Use fixture-managed cleanup withwithSession(sdk, title, callback)for temporary sessions instead of callingsdk.session.delete(...)directly
Prefer theprojectfixture for tests that need a dedicated project with LLM mocking
Usedata-component,data-action, or semantic roles for selectors instead of CSS class names or IDs
UsemodKeyfrom utils for cross-platform keyboard shortcuts (Meta on Mac, Control on Linux/Windows)
In terminal tests, type through the browser usingrunTerminal()andwaitTerminalReady()instead of writing to the PTY through the SDK
Never use wall-clock waits likepage.waitForTimeout(...)to make a test pass
Wait on observable state withexpect(...),expect.poll(...), or existing helpers instead of assuming work is finished after an action
Use locator assertions liketoBeVisible(),toHaveCount(0), andtoHaveAttribute(...)for normal UI state verification
Useexpect.poll(...)for probing mock or backend state rather than transient DOM visibility
Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise in tests
Use direct locators when the interaction is simple and a helper would not add clarity
When validating routing, assert against canonical or resolved workspace slugs using shared helpers from../actionsto account for Windows canonicalization
Test one feature per test file
Callproject.trackSession(sessionID, directory?)andproject.trackDirectory(directory)for any resources created outside the fixture so teardown can clean them up
Files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
🧠 Learnings (15)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:52.535Z
Learning: In Astro-Han/pawwork (`packages/ui/src/theme/context.tsx` and related files), the renaming of localStorage theme keys from `opencode-*` to `pawwork-*` (THEME_ID, COLOR_SCHEME, THEME_CSS_LIGHT, THEME_CSS_DARK) is intentional and should NOT include a migration path from the old keys. Migrating would re-couple PawWork and OpenCode browser storage namespaces, which the PR is explicitly designed to avoid. A reset to the PawWork default theme on upgrade is acceptable by design.
📚 Learning: 2026-04-23T15:26:03.112Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/src/pages/layout/sidebar-items.tsx:102-107
Timestamp: 2026-04-23T15:26:03.112Z
Learning: In Astro-Han/pawwork (`packages/app/src/pages/layout/sidebar-items.tsx`), the `indicator()` function in `SessionRow` intentionally renders `props.leadingSlot` (the pin button) only as a fallback when no status indicator (running/permission/error/unseen) is active. When a higher-priority status wins the slot, the pin button is removed from the DOM — this is a deliberate design choice for the merged leading slot (`#150`). The keyboard unpin path is preserved via: (1) focusing the row anchor triggers `group-focus-within` which reveals the dots menu trigger, then Tab → Enter → "Unpin Session"; (2) the context menu (right-click / Shift+F10) exposes "Unpin Session". The "always render + CSS overlay" approach was considered but rejected due to z-index/pointer-events complexity; residual `...` slot behavior is tracked in `#192`. Do NOT flag the absence of the pin button from the DOM when a status is active as an accessibility regression.
Applied to files:
packages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-23T07:23:23.849Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 180
File: packages/app/src/components/session/session-new-view.tsx:13-18
Timestamp: 2026-04-23T07:23:23.849Z
Learning: In pawwork (Astro-Han/pawwork), prefer using `createStore` instead of multiple `createSignal` calls only when the signals represent **coupled** object state that is updated together (i.e., there is at least one shared batch-update site where the state is changed in the same transaction). If the state fields are **independent** and are mutated by separate handlers (e.g., one handler updates only `selectedSkill` while another updates only `mode`), keep them as individual `createSignal` calls—using `createStore` for truly independent fields adds boilerplate without behavioral benefit.
Applied to files:
packages/app/src/components/prompt-input.tsxpackages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-23T15:10:21.635Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 191
File: packages/app/src/components/session/pawwork-skill-meta.ts:38-39
Timestamp: 2026-04-23T15:10:21.635Z
Learning: This repo configures Tailwind v4 with `--color-*: initial`, which effectively breaks standard Tailwind palette utilities (e.g., `text-violet-500` can resolve to no CSS variable and render as a no-op/black). For brand/accent colors that are not backed by semantic design tokens, use inline styles with the exact hex value (e.g., `style={{ color: '#8B5FBF' }}` / `homeIconStyle: { color: '#8B5FBF' }`) and add a short comment explaining that Tailwind palette utilities won’t work due to the `--color-*: initial` setup. Do not suggest replacing these inline hex colors with Tailwind palette classes anywhere in this repo.
Applied to files:
packages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use direct locators when the interaction is simple and a helper would not add clarity
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use locator assertions like `toBeVisible()`, `toHaveCount(0)`, and `toHaveAttribute(...)` for normal UI state verification
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Prefer fluent helpers and drivers when they make intent obvious and reduce locator-heavy noise in tests
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use `data-component`, `data-action`, or semantic roles for selectors instead of CSS class names or IDs
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use `expect.poll(...)` for probing mock or backend state rather than transient DOM visibility
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/packages/app/src/testing/**/*.ts : Test-only hooks must be inert unless explicitly enabled and should not add normal-runtime listeners, reactive subscriptions, or per-update allocations
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-22T08:49:44.563Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/desktop-electron/src/main/index-sidecar-source.test.ts:3-11
Timestamp: 2026-04-22T08:49:44.563Z
Learning: In `packages/desktop-electron/src/main/index-sidecar-source.test.ts` (Astro-Han/pawwork), the test intentionally uses `expect(source).toContain` / `expect(source).not.toContain` string matching against the raw `index.ts` source text as a lightweight sidecar contract guard. The maintainer has explicitly chosen not to introduce an AST parser (e.g., `babel/parser` or acorn) for this purpose. Do not flag these string-based assertions as fragile or suggest converting them to AST-based matching.
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-20T14:36:04.099Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.099Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use lowercase, descriptive test names (e.g., 'sidebar can be toggled')
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
📚 Learning: 2026-04-23T15:25:27.182Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 193
File: packages/app/e2e/sidebar/sidebar-leading-slot.spec.ts:5-55
Timestamp: 2026-04-23T15:25:27.182Z
Learning: In Astro-Han/pawwork E2E tests (e.g., *.spec.ts under packages/app/e2e), reaching a real "running" session state is not achievable with the bare `sdk` fixture. Use the `project` fixture (to bootstrap the model) and orchestrate the transition with `llm.wait(1)`; even if you set `agent: "build"` and a `system` prompt via `sdk.session.promptAsync`, the current test infrastructure does not trigger an actual LLM call, so it won’t simulate "running" cheaply. Review any attempt to mock/force "running" using only `sdk` as likely ineffective unless it also uses `project` + `llm.wait(1)`.
Applied to files:
packages/app/e2e/prompt/prompt-footer-focus.spec.ts
🔇 Additional comments (7)
packages/util/package.json (1)
3-3: LGTM! Version bump is correct.The patch version increment to
0.2.9aligns with the PR objectives for this release.packages/desktop-electron/package.json (1)
4-4: LGTM! Desktop package version updated correctly.The version increment to
0.2.9is consistent with the other package updates in this release.packages/app/package.json (1)
3-3: LGTM! App package version updated correctly.The patch version bump to
0.2.9is consistent with the release objectives.packages/opencode/package.json (1)
3-3: LGTM! OpenCode package version updated correctly.The version increment to
0.2.9completes the coordinated workspace version bump for this release.packages/app/src/components/prompt-input.tsx (2)
1517-1528: Good file-input reset handling.Resetting
e.currentTarget.valueafter processing files keeps re-selecting the same file working reliably.
1530-1638: Home footer layout fix looks correct for the overlap regression.The single-row
homeModefooter avoids overlapping absolute hit zones while preserving pointer-event gating and non-home fallback behavior.packages/app/e2e/prompt/prompt-footer-focus.spec.ts (1)
43-56: Useful interception diagnostic helper.Center-point hit testing with
data-actioncapture is a solid regression guard for click interception.
Summary
homeMode, so the workspace chip and attach button no longer overlap the model selector hit area0.2.9Why
Release testing found a homepage-only regression after
#191and#193: the model selector looked clickable but did nothing because the left footer controls were intercepting the click target. In-session model switching still worked, so the fix stays scoped to the home composer layout.Related Issue
No standalone issue. This was found during release testing after
#191and#193merged.How To Verify
Manual check:
New sessionLing 2.6 Flash FreetoHy3 preview FreeScreenshots or Recordings
/Users/yuhan/Downloads/pawwork-home-e2e-2026-04-24.pngChecklist
dev, and my PR title and commit messages use Conventional Commits in EnglishMaintainer labeling is fine for this release PR.
Notes:
bun.lockandpackages/opencode/src/provider/models-snapshot.jsfrom dependency install and predev generation. Those changes are not part of the two pushed commits.Summary by CodeRabbit
Refactor
Tests
Chores