refactor: composer and message surface polish after #208#217
Merged
Conversation
Astro-Han
added
enhancement
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughUpdates adjust prompt-input/control sizing and transitions, forward a style prop to WorkspaceChip, switch SendButton to brand theme tokens and shadow CSS variable, add brand color variables and theme wiring, tweak composer max-widths and message radii/whitespace, modify commitlint rules, and add E2E layout/rendering tests. Changes
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)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements UI layout adjustments and introduces a new brand color token system. Key changes include updating widths in PromptInput and session views, adding a style prop to WorkspaceChip, and replacing hardcoded colors in SendButton with CSS variables. Review feedback highlights the need to include the new shadow token in the color generator source and to avoid manual edits in generated CSS files to ensure consistency.
Astro-Han
commented
Apr 24, 2026
Owner
Author
Astro-Han
left a comment
Astro-Han
left a comment
There was a problem hiding this comment.
Focused review on test sufficiency and regression risk. I left a few nitpicky inline comments where current coverage would let the intended visual behavior regress silently.
Astro-Han
added a commit
that referenced
this pull request
Apr 24, 2026
Self-review on PR #217 flagged four behavior changes that manual eyeball verification cannot protect long-term. Adding focused E2E assertions as regression guardrails while keeping manual verification for effect / design approval. - home.spec.ts: assert model chip trigger renders at width 176px (w-44) and all chip row neighbors (attach, variant, workspace, send) stay visible at md viewport. Catches w-44 revert to w-48 or any class change that triggers overflow. - session.spec.ts: set viewport to 1600x1000 and assert the session composer's rendered width stays below or equal to 920px. Catches regressions where the 2xl max-w cap is removed, changed to the message-timeline value (1000px), or otherwise undone. This closes the 'untested 2xl path' gap the PR description explicitly flagged. - session-composer-dock.spec.ts (question text pre-wrap): seed a question with '\\n\\n' in the source string and assert [data-slot="question-text"] has computed white-space: pre-wrap. Catches a future CSS refactor silently dropping pre-wrap and collapsing source newlines into a run-on paragraph. - session-composer-dock.spec.ts (question submit brand color): seed the default question, pick the first option, and assert the submit button's computed background-color is rgb(229, 106, 46). Catches a regression to opencode's --button-primary-base (#e8533a) when the pawwork alias --button-primary-base: var(--button-brand-base) is removed or overridden. This asserts the alias through an actual variant=primary Button surface (not just SendButton), which was the P2 concern from self-review.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/app/home.spec.ts`:
- Line 102: Update the test assertion to check the action-level selector instead
of the wrapper: replace the
locator('[data-component="prompt-variant-control"]').first() visibility
assertion with locator('[data-action="prompt-model-variant"]').first() so the
test verifies the actual clickable trigger; keep the same toBeVisible()
assertion and test flow but target '[data-action="prompt-model-variant"]' for
stronger guarding of the variant chip.
In `@packages/app/e2e/session/session-composer-dock.spec.ts`:
- Around line 662-678: Rename the test constant multilineQuestions to
SCREAMING_SNAKE_CASE (e.g., MULTILINE_QUESTIONS) wherever it is declared and
referenced in this test block (references include the constant used in
withDockSession callback, in llm.toolMatch call, and in seedSessionQuestion).
Ensure all occurrences inside the async test (including the array literal
assignment and usages passed as the questions property) are updated to the new
identifier to keep the test consistent with the coding guideline.
🪄 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: 5d142abb-e75b-4602-a3d7-a19843c2c0e8
📒 Files selected for processing (3)
packages/app/e2e/app/home.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/session/session-composer-dock.spec.ts
📜 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)
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-app
- GitHub Check: unit-desktop
- GitHub Check: unit-opencode
- GitHub Check: unit-app
- GitHub Check: typecheck
- GitHub Check: smoke-macos-arm64
- GitHub Check: analyze-js-ts
- GitHub Check: e2e-artifacts
🧰 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/e2e/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.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/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
🧠 Learnings (22)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:56.086Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:36.205Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, the Model chip trigger button carries `data-action="prompt-model"` (around line 1187) and the Variant chip trigger button carries `data-action="prompt-model-variant"` (around line 1231), both set via `triggerProps`. These are therefore already captured by any `[data-action]` selector sweep in E2E tests and do not need a separate `[data-component]` query to be included in parity assertions — though unioning both is kept as belt-and-suspenders in `collectBarSet`.
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/app/e2e/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-24T05:48:36.205Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:36.205Z
Learning: In E2E parity tests, prefer using the existing `[data-action]` coverage when asserting UI parity. For elements whose trigger props set `data-action` (e.g., `data-action="prompt-model"` and `data-action="prompt-model-variant"` on prompt input chip triggers), you generally do not need to add separate assertions driven by `[data-component]` for parity. Avoid duplicating component-specific queries when the `[data-action]` selector sweep already includes the elements; any extra unioning of selectors should be treated as optional belt-and-suspenders rather than required.
Applied to files:
packages/app/e2e/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.ts
📚 Learning: 2026-04-24T05:39:56.086Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:56.086Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Applied to files:
packages/app/e2e/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test files should be named with the pattern `feature-name.spec.ts`
Applied to files:
packages/app/e2e/app/home.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/app/e2e/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Prefer the `project` fixture for tests that need a dedicated project with LLM mocking
Applied to files:
packages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-24T00:02:50.599Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 203
File: packages/app/e2e/sidebar/sidebar-session-links.spec.ts:34-55
Timestamp: 2026-04-24T00:02:50.599Z
Learning: For Astro-Han/pawwork E2E tests under packages/app/e2e/**/*.spec.ts, do not call project.trackDirectory() or project.trackSession() before project.open() has run. The project fixture throws until open() initializes internal state. Use this ordering pattern: (1) call project.trackSession(sessionID) inside the beforeGoto callback (where state is already available), (2) call project.trackDirectory(directory) and any cross-workspace tracking like project.trackSession(id, directory) immediately after project.open() returns, and (3) if you create any resources before open() that cannot yet be tracked via the fixture, ensure you clean them up explicitly in finally blocks (e.g., cleanupSession / cleanupTestProject).
Applied to files:
packages/app/e2e/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-24T03:51:54.050Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:54.050Z
Learning: In Astro-Han/pawwork E2E tests under packages/app/e2e, do not manually call `project.trackSession(sessionID)` when you obtain a `sessionID` via `project.prompt(text)`. The `project.prompt()` implementation already registers `trackSession(next.sessionID, active.directory)` automatically after the prompt submission is observed and the active session is resolved, so calling `project.trackSession(sessionID)` again will create duplicate session ownership/teardown handling.
Applied to files:
packages/app/e2e/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.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/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 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/e2e/app/home.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-22T08:49:47.800Z
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:47.800Z
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/session/session-composer-dock.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : In terminal tests, type through the browser using `runTerminal()` and `waitTerminalReady()` instead of writing to the PTY through the SDK
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use fixture-managed cleanup with `withSession(sdk, title, callback)` for temporary sessions instead of calling `sdk.session.delete(...)` directly
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/session/session-composer-dock.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Call `project.trackSession(sessionID, directory?)` and `project.trackDirectory(directory)` for any resources created outside the fixture so teardown can clean them up
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use camelCase for variable names in tests
Applied to files:
packages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use SCREAMING_SNAKE_CASE for constants in tests
Applied to files:
packages/app/e2e/app/session.spec.ts
🔇 Additional comments (2)
packages/app/e2e/session/session-composer-dock.spec.ts (1)
689-713: Strong regression coverage for brand token aliasing.This test correctly protects the question-dock submit button against theme-token regressions beyond the prompt send button path.
packages/app/e2e/app/session.spec.ts (1)
50-68: 2xl composer-width guard is well targeted.The viewport setup plus
boundingBox().width <= 920assertion provides a clear regression check for the intended 2xl inset behavior.
Astro-Han
added a commit
that referenced
this pull request
Apr 24, 2026
Self-review on PR #217 flagged four behavior changes that manual eyeball verification cannot protect long-term. Adding three focused E2E assertions as behavior-layer regression guardrails while keeping manual verification for effect and visual polish. Color assertions are intentionally excluded — E2E is the wrong tool for color regression (brittle under theme switches, wrong signal shape vs true visual regression testing), and PawWork does not run toHaveScreenshot() visual regression. The existing home.spec.ts brand-orange assertion is also dropped for the same reason. - home.spec.ts (new): assert model chip trigger renders at width 176px (w-44) and all chip row neighbors (attach, variant, workspace, send) stay visible at md viewport. Catches w-44 revert to w-48 and any class change that triggers overflow into neighbors. - home.spec.ts (removed): the 'brand orange send' test dropped its toHaveCSS background-color rgb assertion. The rest of the test still verifies structural behavior (no DockTray, send button visible and enabled, WorkspaceChip present). Color verification is intentionally manual or left to future visual-regression infrastructure, not baked into behavior E2E. - session.spec.ts (new): set viewport to 1600x1000 and assert the session composer's rendered width stays below or equal to 920px. Catches regressions where the 2xl max-w cap is removed, reverted to the message-timeline value (1000px), or otherwise undone. Closes the 'untested 2xl path' gap the PR description explicitly flagged. - session-composer-dock.spec.ts (new): seed a question whose source contains paragraph breaks (\n\n) and assert the rendered innerText splits into multiple non-empty lines AND that both paragraph strings are present. Tests the visible behavior (paragraph breaks survive render) rather than the CSS mechanism, so the test remains valid if the implementation switches from white-space: pre-wrap to another technique that also preserves line breaks.
Astro-Han
force-pushed
the
fix/model-chip-width-w44
branch
from
042fe05 to
e6cb3cd
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/app/e2e/session/session-composer-dock.spec.ts (1)
666-682:⚠️ Potential issue | 🟡 MinorRename test constant to SCREAMING_SNAKE_CASE (Line 666).
multilineQuestionsis a test constant and should be uppercase for consistency with test naming rules.Suggested patch
- const multilineQuestions = [ + const MULTILINE_QUESTIONS = [ @@ - await llm.toolMatch(inputMatch({ questions: multilineQuestions }), "question", { - questions: multilineQuestions, + await llm.toolMatch(inputMatch({ questions: MULTILINE_QUESTIONS }), "question", { + questions: MULTILINE_QUESTIONS, }) - await seedSessionQuestion(project.sdk, { sessionID: session.id, questions: multilineQuestions }) + await seedSessionQuestion(project.sdk, { sessionID: session.id, questions: MULTILINE_QUESTIONS })As per coding guidelines, "Use SCREAMING_SNAKE_CASE for constants in tests."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/e2e/session/session-composer-dock.spec.ts` around lines 666 - 682, Rename the test constant multilineQuestions to SCREAMING_SNAKE_CASE (e.g., MULTILINE_QUESTIONS) and update all references to it in this spec: the declaration (const multilineQuestions → const MULTILINE_QUESTIONS) and usages in llm.toolMatch(..., { questions: multilineQuestions }) and seedSessionQuestion(..., { questions: multilineQuestions }) (and any other occurrences) so the identifier is consistent throughout the test.packages/app/e2e/app/home.spec.ts (1)
100-100: 🧹 Nitpick | 🔵 TrivialAssert the variant trigger instead of only the wrapper.
Line 100 checks
[data-component="prompt-variant-control"], which can still pass if the clickable trigger regresses. Prefer asserting[data-action="prompt-model-variant"].Suggested patch
- await expect(composer.locator('[data-component="prompt-variant-control"]').first()).toBeVisible() + await expect(composer.locator('[data-action="prompt-model-variant"]').first()).toBeVisible()Based on learnings, parity assertions should prefer existing
[data-action]coverage for model/variant triggers when available.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/e2e/app/home.spec.ts` at line 100, Change the e2e assertion to target the actual clickable trigger element instead of the wrapper: update the Composer test that uses composer.locator('[data-component="prompt-variant-control"]').first() to assert visibility (or visibility/clickability) of the trigger selector '[data-action="prompt-model-variant"]' using the same composer.locator(...) API so the test fails if the clickable trigger regresses (refer to composer.locator and the '[data-component="prompt-variant-control"]' selector to locate where to change).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/app/e2e/app/home.spec.ts`:
- Line 100: Change the e2e assertion to target the actual clickable trigger
element instead of the wrapper: update the Composer test that uses
composer.locator('[data-component="prompt-variant-control"]').first() to assert
visibility (or visibility/clickability) of the trigger selector
'[data-action="prompt-model-variant"]' using the same composer.locator(...) API
so the test fails if the clickable trigger regresses (refer to composer.locator
and the '[data-component="prompt-variant-control"]' selector to locate where to
change).
In `@packages/app/e2e/session/session-composer-dock.spec.ts`:
- Around line 666-682: Rename the test constant multilineQuestions to
SCREAMING_SNAKE_CASE (e.g., MULTILINE_QUESTIONS) and update all references to it
in this spec: the declaration (const multilineQuestions → const
MULTILINE_QUESTIONS) and usages in llm.toolMatch(..., { questions:
multilineQuestions }) and seedSessionQuestion(..., { questions:
multilineQuestions }) (and any other occurrences) so the identifier is
consistent throughout the test.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9e37bed5-cab9-4c47-83c3-2f296603f603
📒 Files selected for processing (3)
packages/app/e2e/app/home.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/session/session-composer-dock.spec.ts
📜 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)
- GitHub Check: unit-opencode
- GitHub Check: unit-app
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: typecheck
- GitHub Check: unit-windows-app
- GitHub Check: unit-desktop
- GitHub Check: smoke-macos-arm64
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 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/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.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/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
🧠 Learnings (21)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:56.086Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:36.205Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, the Model chip trigger button carries `data-action="prompt-model"` (around line 1187) and the Variant chip trigger button carries `data-action="prompt-model-variant"` (around line 1231), both set via `triggerProps`. These are therefore already captured by any `[data-action]` selector sweep in E2E tests and do not need a separate `[data-component]` query to be included in parity assertions — though unioning both is kept as belt-and-suspenders in `collectBarSet`.
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-22T08:49:47.800Z
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:47.800Z
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/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-24T03:51:54.050Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:54.050Z
Learning: In Astro-Han/pawwork E2E tests under packages/app/e2e, do not manually call `project.trackSession(sessionID)` when you obtain a `sessionID` via `project.prompt(text)`. The `project.prompt()` implementation already registers `trackSession(next.sessionID, active.directory)` automatically after the prompt submission is observed and the active session is resolved, so calling `project.trackSession(sessionID)` again will create duplicate session ownership/teardown handling.
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test files should be named with the pattern `feature-name.spec.ts`
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use SCREAMING_SNAKE_CASE for constants in tests
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use camelCase for variable names in tests
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-24T00:02:50.599Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 203
File: packages/app/e2e/sidebar/sidebar-session-links.spec.ts:34-55
Timestamp: 2026-04-24T00:02:50.599Z
Learning: For Astro-Han/pawwork E2E tests under packages/app/e2e/**/*.spec.ts, do not call project.trackDirectory() or project.trackSession() before project.open() has run. The project fixture throws until open() initializes internal state. Use this ordering pattern: (1) call project.trackSession(sessionID) inside the beforeGoto callback (where state is already available), (2) call project.trackDirectory(directory) and any cross-workspace tracking like project.trackSession(id, directory) immediately after project.open() returns, and (3) if you create any resources before open() that cannot yet be tracked via the fixture, ensure you clean them up explicitly in finally blocks (e.g., cleanupSession / cleanupTestProject).
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.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/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 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/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-24T05:48:36.205Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:36.205Z
Learning: In E2E parity tests, prefer using the existing `[data-action]` coverage when asserting UI parity. For elements whose trigger props set `data-action` (e.g., `data-action="prompt-model"` and `data-action="prompt-model-variant"` on prompt input chip triggers), you generally do not need to add separate assertions driven by `[data-component]` for parity. Avoid duplicating component-specific queries when the `[data-action]` selector sweep already includes the elements; any extra unioning of selectors should be treated as optional belt-and-suspenders rather than required.
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use fixture-managed cleanup with `withSession(sdk, title, callback)` for temporary sessions instead of calling `sdk.session.delete(...)` directly
Applied to files:
packages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Call `project.trackSession(sessionID, directory?)` and `project.trackDirectory(directory)` for any resources created outside the fixture so teardown can clean them up
Applied to files:
packages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-24T05:39:56.086Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:56.086Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Applied to files:
packages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.ts
🔇 Additional comments (2)
packages/app/e2e/app/session.spec.ts (1)
50-68: Strong 2xl regression guard for composer width.The assertion on Line 66 cleanly protects the intended
2xlmax-width behavior through rendered layout, not just class presence.packages/app/e2e/app/home.spec.ts (1)
86-99: Good regression coverage for the w-44 chip row.This test is a solid guard against width regressions and neighbor-control overflow in the unified home composer row.
Also applies to: 101-103
Tighten the model chip pill proportion and align its hover animation with the sibling variant / workspace chips. Width: w-48 (192px, 6:1 pill ratio) was visibly wider than the rest of the composer row once short model names like 'Kimi K2.6' or 'GLM 5.1' were the typical label. The label then sat in ~46px of centered whitespace on each side, reading as a hollow chip. Tighten to w-44 (176px, 5.5:1 pill ratio) — still fixed-width (avoids row reflow on model switch), but the centered whitespace drops to ~38px and the chip reads as filled. The longest bundled model name 'Doubao Seed 2.0 Code' (20 chars) still fits without truncation. Hover transition: sibling chips (variant, workspace) use 'transition-colors' so their background cross-fades over ~150ms on hover. The model chip was missing this class, so its hover background swap was instant. Scanning across the composer row exposed the inconsistency. Adding transition-colors makes all three chips share the same hover animation.
Two coordinated layout adjustments so the composer reads as a floating input surface rather than as a wall-to-wall band. Home composer: session-new-view.tsx wrapper bumped from max-w-[640px] to max-w-[760px] during the PR #208 composer unification to fit the worst-case one-row chip layout. After the model chip narrowed to w-44 (d27bc88) the worst-case budget (attach 32 + gaps 24 + model 176 + variant 140 + workspace 188 + gap-8 32 + send 32 = 624 plus 32 inset = 656) fits comfortably in 688 available at max-w-[720px] with 32px slack. Tighten from 760 to 720 so the home composer no longer floats in empty horizontal space on wider windows. Session composer: session-composer-region.tsx shared the message-timeline's responsive 'md:max-w-200 md:mx-auto 2xl:max-w-[1000px]' so the composer sat edge-to-edge with the messages above it. Subtract 80px at each breakpoint: 'md:max-w-[720px] 2xl:max-w-[920px]'. The composer now insets 40px from each side of its message column and matches the home composer width (720) at typical md viewports. Below md, both the messages and the composer continue to fill the viewport, so nothing changes there.
In the unified composer (home mode) the attach / model / variant chips all receive a motion style sourced from the buttonsSpring memo ('buttons()'), so they share the opacity / scale / blur exit animation when the editor switches into shell mode. WorkspaceChip did not accept a style prop and had no way to participate in that spring — it disappeared abruptly from the DOM on mode switch while its neighbors smoothly faded.
Add an optional 'style' prop to WorkspaceChip that flows into the Popover triggerProps, and pass 'buttons()' from the composer usage site. In normal (non-shell) mode the buttons() memo resolves to { opacity: 1, transform: scale(1), filter: blur(0), pointer-events: auto } — a no-op — so everyday rendering is unchanged. On shell-mode entry WorkspaceChip now fades in sync with the rest of the chip row.
Replace the SendButton's hardcoded brand orange (#E56A2E base, #D05E26 hover, rgba(229, 106, 46, 0.45) shadow) with a proper design-token system so future brand-orange surfaces consume one source of truth instead of redeclaring the hex. Token definition (theme.css): - Three new raw tokens at :root so they resolve under any theme, not only pawwork. Brand color is brand color; it should not vanish if a user switches themes. --button-brand-base: #e56a2e --button-brand-hover: #d05e26 --button-brand-shadow: rgba(229, 106, 46, 0.45) - Alias --button-primary-base to var(--button-brand-base) inside :root[data-theme='pawwork']. Because the pawwork theme selector's specificity (0,1,1) outranks the base :root's (0,0,1) and the nested dark @media, the alias applies to both light and dark mode under pawwork. Every variant='primary' Button in PawWork (settings save, confirmation dialogs, question-dock submit, ...) now resolves to the brand orange, matching SendButton. Tailwind wiring (colors.txt, colors.css): - Register --button-brand-base and --button-brand-hover in colors.txt (the generator source for script/tailwind.ts). - colors.css regenerates to map --color-button-brand-base / --color-button-brand-hover → var(--button-brand-*), exposing the 'bg-button-brand-base' / 'hover:bg-button-brand-hover' utility classes the same way --button-primary / --button-secondary are exposed. - --button-brand-shadow is intentionally NOT registered in colors.txt because it is a composite CSS value (0 1px 3px <rgba>), not a single color primitive. Shadow is consumed via Tailwind's arbitrary-value syntax instead. Consumer (send-button.tsx): - bg / hover switch to 'bg-button-brand-base' / 'hover:bg-button-brand-hover' utility classes. - Shadow uses 'shadow-[0_1px_3px_var(--button-brand-shadow)]' (arbitrary value), which is the standard Tailwind approach for composite shadows that reference a token. - No visible change: tokens resolve to the same hex values. Verified in local Electron dev that the orange renders identically in active state and that disabled state still falls through to --border-weak-base.
Raise border-radius on two message-adjacent surfaces to bring them in line with the rest of the PawWork chat language (composer dock shell at 26px, chips at rounded-full). user-message-text (6px → 12px): the bubble sat boxy next to the 26px composer shell above and a history of rounded-full chips, reading as a hard rectangle. Bumping to 12px gives the bubble a softer chat feel while staying distinctly smaller than the composer shell — preserving the 'container vs content' hierarchy so history does not masquerade as a pile of composers. question-option (6px → 20px): on the much larger option cards a 12px radius is not proportionally enough; it reads the same as the old 6px at a glance because the corner-to-card-height ratio is low. 20px (≈33% of the 60px card height) matches the perceptual roundness of 12px on the smaller message bubble (≈30% ratio), so both surfaces feel equally soft despite the different absolute values.
The [data-slot='question-text'] block had no white-space rule, so the browser default ('normal') collapsed any \n in the source question string into a single run-on paragraph. Question bodies produced by the model often have natural paragraph breaks (setup + options + closing question), but they all rendered as one wall of text, which hurt readability on longer questions.
Set white-space: pre-wrap so any \n already present in the question string renders as a visible line break. This is purely additive — strings without \n behave exactly as before. Does not change font, spacing, or any other typography property.
@commitlint/config-conventional ships body-max-line-length and footer-max-line-length at 100. That 100-char value is a 1990s terminal-readability tradition carried forward — it was already a concession from the stricter 72-char rule. In a solo-dev + AI-agent workflow where commit bodies are read primarily through the GitHub PR UI and VS Code git integration (both soft-wrap), hard-wrapping natural prose paragraphs at any arbitrary character ceiling is friction without a matching readability payoff. An earlier attempt in this same PR raised the ceiling to 500 as a 'soft' guardrail, but two regular commits in this branch went over 500 anyway and tripped the check. That demonstrates the problem: any numeric cap picked for 'catches truly pathological cases' is also above what commitlint's own failure message looks like when it trips, so the rule keeps being friction-only. Set both body-max-line-length and footer-max-line-length to [0] (disabled). The rest of the Conventional Commits structure — type-enum, type-case, subject-case, header-max-length (100), and everything else from config-conventional — is untouched. This is a relaxation of length discipline for body and footer only, not a departure from Conventional Commits. If a future commit body really is pathologically long (pasted stack traces, binary blobs, URL walls), that is a code-review concern, not a lint-time gate.
Astro-Han
force-pushed
the
fix/model-chip-width-w44
branch
from
e6cb3cd to
0eff5dd
Compare
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/app/e2e/app/home.spec.ts (1)
96-102:⚠️ Potential issue | 🟡 MinorUse the actual variant trigger here.
This wrapper-level visibility check can still pass if the clickable
prompt-model-variantcontrol regresses, so it does not fully protect the interaction surface the test is meant to cover.Suggested fix
- await expect(composer.locator('[data-component="prompt-variant-control"]').first()).toBeVisible() + await expect(composer.locator('[data-action="prompt-model-variant"]').first()).toBeVisible()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/e2e/app/home.spec.ts` around lines 96 - 102, The visibility assertions should target the actual variant trigger element instead of the wrapper to ensure the interactive control is present: replace the wrapper-level check for '[data-component="prompt-variant-control"]' with an assertion that the actual variant trigger '[data-action="prompt-model-variant"]' (or its more specific selector) is visible and/or enabled; keep the other checks for '[data-action="prompt-attach"]', the workspace switch button, and '[data-action="prompt-submit"]' unchanged so the test verifies the true clickable control (prompt-model-variant) rather than only its wrapper.packages/app/e2e/session/session-composer-dock.spec.ts (1)
666-682:⚠️ Potential issue | 🟡 MinorRename the test constant to
MULTILINE_QUESTIONS.
multilineQuestionsviolates the test constant naming rule and should be updated at the declaration and all uses.Suggested patch
- const multilineQuestions = [ + const MULTILINE_QUESTIONS = [ @@ - await llm.toolMatch(inputMatch({ questions: multilineQuestions }), "question", { - questions: multilineQuestions, + await llm.toolMatch(inputMatch({ questions: MULTILINE_QUESTIONS }), "question", { + questions: MULTILINE_QUESTIONS, }) - await seedSessionQuestion(project.sdk, { sessionID: session.id, questions: multilineQuestions }) + await seedSessionQuestion(project.sdk, { sessionID: session.id, questions: MULTILINE_QUESTIONS })As per coding guidelines, use SCREAMING_SNAKE_CASE for constants in tests.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/app/e2e/session/session-composer-dock.spec.ts` around lines 666 - 682, Rename the test constant symbol multilineQuestions to MULTILINE_QUESTIONS everywhere it is declared and referenced (including the const declaration and all usages in withDockSession callback, calls to llm.toolMatch, seedSessionQuestion, and any other places in this test file), updating the identifier consistently to follow SCREAMING_SNAKE_CASE for test constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/app/e2e/app/home.spec.ts`:
- Around line 96-102: The visibility assertions should target the actual variant
trigger element instead of the wrapper to ensure the interactive control is
present: replace the wrapper-level check for
'[data-component="prompt-variant-control"]' with an assertion that the actual
variant trigger '[data-action="prompt-model-variant"]' (or its more specific
selector) is visible and/or enabled; keep the other checks for
'[data-action="prompt-attach"]', the workspace switch button, and
'[data-action="prompt-submit"]' unchanged so the test verifies the true
clickable control (prompt-model-variant) rather than only its wrapper.
In `@packages/app/e2e/session/session-composer-dock.spec.ts`:
- Around line 666-682: Rename the test constant symbol multilineQuestions to
MULTILINE_QUESTIONS everywhere it is declared and referenced (including the
const declaration and all usages in withDockSession callback, calls to
llm.toolMatch, seedSessionQuestion, and any other places in this test file),
updating the identifier consistently to follow SCREAMING_SNAKE_CASE for test
constants.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 72b596cc-77a3-45ec-adc5-57a3ce7b5279
📒 Files selected for processing (13)
.commitlintrc.jsonpackages/app/e2e/app/home.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/session/session-composer-dock.spec.tspackages/app/src/components/prompt-input.tsxpackages/app/src/components/prompt-input/send-button.tsxpackages/app/src/components/prompt-input/workspace-chip.tsxpackages/app/src/components/session/session-new-view.tsxpackages/app/src/pages/session/composer/session-composer-region.tsxpackages/ui/script/colors.txtpackages/ui/src/components/message-part.csspackages/ui/src/styles/tailwind/colors.csspackages/ui/src/styles/theme.css
📜 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)
- GitHub Check: smoke-macos-arm64
- GitHub Check: unit-windows-opencode-server-tools
- GitHub Check: unit-windows-opencode-session
- GitHub Check: unit-windows-desktop
- GitHub Check: unit-windows-opencode-config-project
- GitHub Check: unit-windows-app
- GitHub Check: unit-app
- GitHub Check: unit-opencode
- GitHub Check: typecheck
- GitHub Check: unit-desktop
- GitHub Check: e2e-artifacts
- GitHub Check: analyze-js-ts
🧰 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/session/session-new-view.tsxpackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/src/components/prompt-input/send-button.tsxpackages/app/e2e/app/home.spec.tspackages/app/src/components/prompt-input/workspace-chip.tsxpackages/app/src/components/prompt-input.tsx
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/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
🧠 Learnings (30)
📓 Common learnings
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:56.086Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:36.205Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, the Model chip trigger button carries `data-action="prompt-model"` (around line 1187) and the Variant chip trigger button carries `data-action="prompt-model-variant"` (around line 1231), both set via `triggerProps`. These are therefore already captured by any `[data-action]` selector sweep in E2E tests and do not need a separate `[data-component]` query to be included in parity assertions — though unioning both is kept as belt-and-suspenders in `collectBarSet`.
📚 Learning: 2026-04-24T05:39:56.086Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/src/components/prompt-input.tsx:1569-1611
Timestamp: 2026-04-24T05:39:56.086Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, after the composer unification in PR `#208` (fixed in commit 5d810aa):
- `SendButton.disabled` does NOT gate on `store.mode !== "normal"`. Shell mode has a fully visible, clickable orange submit button that calls `handleSubmit` directly (same path as the Enter key in `handleKeyDown`). Do NOT suggest re-adding the mode gate.
- `SendButton` does NOT use the `buttons()` spring opacity animation (`style={buttons()}`). It is always fully visible regardless of mode.
- `WorkspaceChip` is gated on `props.homeMode && store.mode === "normal"` so it hides in shell mode (preventing it from appearing isolated/bright while neighboring controls fade).
- The left-side chip group (`aria-hidden={store.mode !== "normal"}`) covers attach/model/variant/workspace controls only; `SendButton` remains in a separate right-side sibling div.
Applied to files:
packages/app/src/components/session/session-new-view.tsxpackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/components/prompt-input/send-button.tsxpackages/ui/script/colors.txtpackages/app/e2e/app/home.spec.tspackages/ui/src/styles/tailwind/colors.csspackages/app/src/components/prompt-input/workspace-chip.tsxpackages/ui/src/components/message-part.csspackages/ui/src/styles/theme.csspackages/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/session/session-new-view.tsxpackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/src/components/prompt-input/send-button.tsxpackages/app/e2e/app/home.spec.tspackages/app/src/components/prompt-input/workspace-chip.tsxpackages/app/src/components/prompt-input.tsx
📚 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/session/session-new-view.tsxpackages/app/src/pages/session/composer/session-composer-region.tsxpackages/app/src/components/prompt-input/send-button.tsxpackages/app/src/components/prompt-input/workspace-chip.tsxpackages/app/src/components/prompt-input.tsx
📚 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, allow intentional string-based assertions that use expect(...).toContain / expect(...).not.toContain against the raw index.ts source text as a lightweight sidecar contract guard. Do not review these as “fragile” or recommend switching to AST-based matching (e.g., babel/parser/acorn) when this is explicitly chosen for this purpose.
Applied to files:
.commitlintrc.json
📚 Learning: 2026-04-24T03:51:54.050Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 206
File: packages/app/e2e/prompt/prompt-footer-focus.spec.ts:131-143
Timestamp: 2026-04-24T03:51:54.050Z
Learning: In Astro-Han/pawwork E2E tests under packages/app/e2e, do not manually call `project.trackSession(sessionID)` when you obtain a `sessionID` via `project.prompt(text)`. The `project.prompt()` implementation already registers `trackSession(next.sessionID, active.directory)` automatically after the prompt submission is observed and the active session is resolved, so calling `project.trackSession(sessionID)` again will create duplicate session ownership/teardown handling.
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test one feature per test file
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.tspackages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-22T08:49:47.800Z
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:47.800Z
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/session/session-composer-dock.spec.tspackages/app/e2e/app/home.spec.tspackages/ui/src/components/message-part.css
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Call `project.trackSession(sessionID, directory?)` and `project.trackDirectory(directory)` for any resources created outside the fixture so teardown can clean them up
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.tspackages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use fixture-managed cleanup with `withSession(sdk, title, callback)` for temporary sessions instead of calling `sdk.session.delete(...)` directly
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : In terminal tests, type through the browser using `runTerminal()` and `waitTerminalReady()` instead of writing to the PTY through the SDK
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Prefer the `project` fixture for tests that need a dedicated project with LLM mocking
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use SCREAMING_SNAKE_CASE for constants in tests
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Use camelCase for variable names in tests
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-24T00:02:50.599Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 203
File: packages/app/e2e/sidebar/sidebar-session-links.spec.ts:34-55
Timestamp: 2026-04-24T00:02:50.599Z
Learning: For Astro-Han/pawwork E2E tests under packages/app/e2e/**/*.spec.ts, do not call project.trackDirectory() or project.trackSession() before project.open() has run. The project fixture throws until open() initializes internal state. Use this ordering pattern: (1) call project.trackSession(sessionID) inside the beforeGoto callback (where state is already available), (2) call project.trackDirectory(directory) and any cross-workspace tracking like project.trackSession(id, directory) immediately after project.open() returns, and (3) if you create any resources before open() that cannot yet be tracked via the fixture, ensure you clean them up explicitly in finally blocks (e.g., cleanupSession / cleanupTestProject).
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.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/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-24T05:48:36.205Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:36.205Z
Learning: In E2E parity tests, prefer using the existing `[data-action]` coverage when asserting UI parity. For elements whose trigger props set `data-action` (e.g., `data-action="prompt-model"` and `data-action="prompt-model-variant"` on prompt input chip triggers), you generally do not need to add separate assertions driven by `[data-component]` for parity. Avoid duplicating component-specific queries when the `[data-action]` selector sweep already includes the elements; any extra unioning of selectors should be treated as optional belt-and-suspenders rather than required.
Applied to files:
packages/app/e2e/session/session-composer-dock.spec.tspackages/app/e2e/app/session.spec.tspackages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Import test utilities from `../fixtures` instead of `playwright/test`
Applied to files:
packages/app/e2e/app/session.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/session.spec.tspackages/app/e2e/app/home.spec.tspackages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-24T05:48:36.205Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 208
File: packages/app/e2e/app/composer-parity.spec.ts:0-0
Timestamp: 2026-04-24T05:48:36.205Z
Learning: In Astro-Han/pawwork `packages/app/src/components/prompt-input.tsx`, the Model chip trigger button carries `data-action="prompt-model"` (around line 1187) and the Variant chip trigger button carries `data-action="prompt-model-variant"` (around line 1231), both set via `triggerProps`. These are therefore already captured by any `[data-action]` selector sweep in E2E tests and do not need a separate `[data-component]` query to be included in parity assertions — though unioning both is kept as belt-and-suspenders in `collectBarSet`.
Applied to files:
packages/app/src/components/prompt-input/send-button.tsxpackages/app/src/components/prompt-input/workspace-chip.tsxpackages/ui/src/styles/theme.csspackages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-23T15:10:25.201Z
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:25.201Z
Learning: In Astro-Han/pawwork, Tailwind v4 is configured with `--color-*: initial`, which resets the entire default palette. This means standard Tailwind color utilities like `text-violet-500` resolve to no CSS variable and render black (i.e., they are a no-op). For accent/brand colors that do not have a semantic design token (e.g., the violet writing-assistant accent `#8B5FBF`), use an inline style (e.g., `homeIconStyle: { color: "#8B5FBF" }`) and document the reason with a comment. Do NOT suggest replacing inline hex colors with Tailwind palette utilities in this repo.
Applied to files:
packages/ui/script/colors.txtpackages/ui/src/styles/tailwind/colors.csspackages/ui/src/styles/theme.css
📚 Learning: 2026-04-23T17:02:39.474Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 198
File: packages/ui/src/theme/themes/pawwork.json:32-33
Timestamp: 2026-04-23T17:02:39.474Z
Learning: In Astro-Han/pawwork, `#FF5910` is the locked PawWork brand orange used for `button-primary-base` (and `--accent-brand` in `index.css`, theme token `interactive`/`primary`). The `text-on-interactive-base: `#FFFFFF`` on `button-primary-base: `#FF5910`` combination produces a contrast ratio of ~3.14:1. This intentionally fails WCAG AA normal-text (4.5:1) but passes AA non-text (SC 1.4.11, 3:1) and AA large-text (SC 1.4.3, 3:1). The tradeoff is accepted for brand consistency; do NOT flag this contrast pair as a WCAG violation in future reviews. If a specific surface requires stricter contrast, it should be addressed individually (e.g., larger text, ghost variant, outline button) rather than changing the palette.
Applied to files:
packages/ui/script/colors.txtpackages/ui/src/styles/tailwind/colors.csspackages/ui/src/styles/theme.css
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.tspackages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.tspackages/app/src/components/prompt-input.tsx
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
Learning: Applies to packages/app/e2e/**/*.spec.ts : Test files should be named with the pattern `feature-name.spec.ts`
Applied to files:
packages/app/e2e/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.ts
📚 Learning: 2026-04-20T14:36:04.113Z
Learnt from: CR
Repo: Astro-Han/pawwork PR: 0
File: packages/app/e2e/AGENTS.md:0-0
Timestamp: 2026-04-20T14:36:04.113Z
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/app/home.spec.ts
📚 Learning: 2026-04-23T17:02:35.873Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 198
File: packages/app/src/index.css:95-97
Timestamp: 2026-04-23T17:02:35.873Z
Learning: In the pawwork repository, Stylelint is intentionally not configured (per AGENTS.md, only linting is enforced and the Biome formatter is disabled). When reviewing CSS files, do not raise Stylelint rule-based issues (e.g., `declaration-empty-line-before`) because they are false positives and not enforced by the project toolchain.
Applied to files:
packages/ui/src/styles/tailwind/colors.csspackages/ui/src/components/message-part.csspackages/ui/src/styles/theme.css
📚 Learning: 2026-04-22T09:32:58.310Z
Learnt from: Astro-Han
Repo: Astro-Han/pawwork PR: 126
File: packages/ui/src/theme/context.tsx:11-16
Timestamp: 2026-04-22T09:32:58.310Z
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.
Applied to files:
packages/ui/src/styles/theme.csspackages/app/src/components/prompt-input.tsx
🪛 Biome (2.4.12)
packages/ui/src/styles/tailwind/colors.css
[error] 4-237: Tailwind-specific syntax is disabled.
(parse)
🔇 Additional comments (11)
.commitlintrc.json (1)
3-7: Commitlint override is valid and scoped clearly.The config cleanly extends conventional rules while explicitly disabling only body/footer line-length enforcement.
packages/app/src/components/session/session-new-view.tsx (1)
57-59: Looks good.The
max-w-[720px]home wrapper matches the updated composer sizing contract and keeps the layout centered as intended.packages/app/src/pages/session/composer/session-composer-region.tsx (1)
158-159: Looks good.The
720px / 920pxcentered cap matches the updated home/session layout split and the intended 2xl inset.packages/app/e2e/app/session.spec.ts (1)
50-68: Good regression coverage.The 2xl viewport width assertion exercises the updated breakpoint contract and should catch the intended dock-width regression.
packages/ui/src/components/message-part.css (1)
128-136: Looks good.The larger bubble radii and preserved question whitespace match the intended message/question polish and the new newline-rendering coverage.
Also applies to: 914-957
packages/ui/script/colors.txt (1)
99-100: Looks good.These brand token values mirror the theme layer and keep the button color source of truth consistent.
packages/app/src/components/prompt-input/send-button.tsx (1)
23-29: Looks good.The button now uses the shared brand tokens cleanly, and the disabled-state behavior remains unchanged.
packages/app/src/components/prompt-input.tsx (1)
1144-1187: Looks good.The tighter
w-44model chip and the sharedbuttons()styling onWorkspaceChipmatch the intended chip-row polish and keep the home shell behavior consistent.Also applies to: 1603-1605
packages/app/src/components/prompt-input/workspace-chip.tsx (1)
16-137: Looks good.The new
stylepassthrough is consistent with the caller update, and the eager workspace fetch matches the intended pre-open behavior.packages/ui/src/styles/tailwind/colors.css (1)
104-105: Looks good.These token mappings stay in sync with the new
--button-brand-*variables and support the updated brand button styling chain.packages/ui/src/styles/theme.css (1)
198-200: Looks good.The new brand variables and the pawwork
--button-primary-basealias line up with the send-button brand unification.Also applies to: 616-616
Self-review on this PR flagged four behavior surfaces where manual eyeball verification cannot protect long-term. Adding three focused E2E assertions as behavior-layer guardrails while keeping manual verification for effect and visual polish. Color assertions are intentionally excluded — E2E is the wrong tool for color regression because toHaveCSS('background-color', ...) breaks under any theme switch (brittle), and color belongs to visual-regression testing via toHaveScreenshot() pixel baselines, which PawWork does not run.
home.spec.ts (new): assert the model chip trigger renders at width 176px (w-44) and that all chip-row neighbors — attach, variant, workspace, send — stay visible at the default md viewport. Catches a revert to w-48, any class change that resizes the chip, or any neighbor getting pushed out of the row.
home.spec.ts (removed): the pre-existing 'brand orange send' test dropped its toHaveCSS('background-color', 'rgb(229, 106, 46)') line. The rest of the test still verifies structural behavior (no DockTray, send button visible and enabled, WorkspaceChip present). Color verification is intentionally manual or left to future visual-regression infrastructure.
session.spec.ts (new): set the viewport to 1600x1000 so the test renders at the 2xl breakpoint, then assert the session composer's rendered width stays below or equal to 920px. Catches regressions where the 2xl max-w cap is removed, reverted to the message-timeline value (1000px), or otherwise undone. Closes the 'untested 2xl path' gap this PR's description explicitly flagged.
session-composer-dock.spec.ts (new): seed a question whose source contains paragraph breaks ('\n\n') and assert the rendered innerText splits into multiple non-empty lines AND that both paragraph strings are present. Tests the visible rendering behavior (paragraph breaks survive render) rather than the CSS mechanism, so the test remains valid if the implementation switches from white-space: pre-wrap to another technique that also preserves line breaks.
Astro-Han
force-pushed
the
fix/model-chip-width-w44
branch
from
0eff5dd to
1271d32
Compare
This was referenced Apr 29, 2026
11 tasks
Astro-Han
added a commit
that referenced
this pull request
May 5, 2026
Root cause: - Issue #455 reported a visible layout regression after v2026.5.5: the session conversation flow could appear narrower than the bottom composer/input bar. - The failed contract was in the session UI layout boundary, not API, persistence, desktop IPC, model harness, or #440 typography tokens. - Existing E2E coverage measured `[data-component=\"session-prompt-dock\"]`, but that node is intentionally full width. The test therefore did not protect the visible composer column/shell that users actually compare with the timeline. Fix: - Added stable selectors for the visible session timeline and composer columns. - Aligned the session composer wrapper inset below `md` with the message container inset (`px-4 md:px-3`), while preserving the home composer's original `px-3` and the #217 desktop/2xl composer inset design. - Replaced the old outer-dock width assertion with a bounding-box regression test that checks the timeline column, composer column, assistant content, and visible composer shell at the 744px regression viewport and the 1600px 2xl viewport. - Kept the fix scoped to `packages/app` session layout and E2E selectors; no token/font/backend/desktop changes. Verification: - Red check before fix: focused E2E failed at 744px with assistant content width + 1 = 717 versus visible composer shell width = 722.5. - `bun --cwd packages/app test:e2e e2e/app/session.spec.ts -g \"session timeline visible content\"` -> 1 passed. - `bun --cwd packages/app test:e2e e2e/app/session.spec.ts` -> 3 passed. - `bun --cwd packages/app typecheck` -> exit 0. - `git diff --check` -> no whitespace errors. - PR checks passed, including e2e-artifacts, typecheck, unit jobs, desktop smoke, CodeQL, and commit/title lint. Review follow-up: - Avoided changing the home composer narrow layout by keeping home at `px-3`. - Added a 2xl assertion that the timeline remains meaningfully wider than the composer, preserving the earlier composer inset intent. - Did not add a 767/768/769 breakpoint matrix because the red-green regression is covered at 744px and the existing 2xl contract is separately covered. Closes #455.
Astro-Han
added a commit
that referenced
this pull request
May 6, 2026
Root cause: - PR #464 fixed the original composer/timeline mismatch by replacing the session timeline medium-width cap with explicit `md:max-w-[800px]`. - The #440 UI slice merge chain later regressed that fix. PR #461 first changed both timeline caps back to `md:max-w-200`, which resolves to about 650px with PawWork's 13px root font. - PR #460 then kept that same `message-timeline.tsx` state while removing dead delete-session code, so #479 surfaced as the composer still appearing wider than the reading column at right-panel-hidden desktop widths. Fix: - Restore both session timeline medium-width caps to `md:max-w-[800px]`. - Keep the existing 2xl timeline cap and composer caps unchanged, preserving the prior #217/#456 intent that composer remains inset at very wide desktop sizes. - Add a focused E2E checkpoint for #479: viewport 1280x900 with `#right-panel[aria-hidden="true"]`, then reuse the existing timeline/composer bounding-box contract. Verification: - Red check: temporarily restored the old `md:max-w-200` cap and confirmed the focused E2E failed at the new 1280px/right-panel-hidden checkpoint with timeline 651px vs composer 720px. - Focused E2E passed: `bun --cwd packages/app test:e2e e2e/app/session.spec.ts -g "session timeline visible content is not narrower than composer shell"`. - Typecheck passed: `bun --cwd packages/app typecheck`. - Diff check passed: `git diff --check`. - PR CI passed, including typecheck, unit-app, unit-opencode, unit-desktop, e2e-artifacts, smoke-macos-arm64, CodeQL, dependency review, commit lint, and PR title lint. Follow-up: - No immediate product follow-up is required for #479. - A separate architecture issue may still be worth opening later for the broader session width contract, because timeline, message item, and composer width values are still split across files and have accumulated breakpoint-specific tests. Closes #479.
This was referenced May 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Visual polish tail of the composer unification landed in #208 (home / composer redesign tracked under #181). Eleven small commits across app and ui, intentionally kept as separate reversible intents so any one can be reverted or cherry-picked.
Model chip shape & motion
d27bc88b1anarrow model chip from w-48 to w-44 for a tighter pill proportion (5:1 → 5.5:1, closer to classic pill range)b0fe762a89addtransition-colorsso hover matches sibling variant / workspace chipsComposer wrapper geometry
f826510fb7tighten home composer wrapper from 760 to 720 (~10% narrower)63d869418ainset session composer 80px from its own message column (720 / 920 responsive) so it reads as a floating input instead of edge-to-edge with history; at typical md windows session and home now render at the same 720pxWorkspace chip fade consistency
fc5e9f816ethread the sharedbuttons()motion spring intoWorkspaceChipso it fades in sync with attach / model / variant on shell-mode toggleBrand button token system
f7924f895cintroduce--button-brand-base / hover / shadowat:rootand map through tailwind@themeviacolors.txt(generator source) →colors.csse3152d4358migrate SendButton from hardcoded#E56A2E / #D05E26to the new utility classesa99d0b6d6calias--button-primary-baseto brand in pawwork theme so everyvariant="primary"Button (question tool submit, settings save, confirmation CTAs) renders in the same brand orange as SendButtonMessage + question tool radius softening
9086410f65user message bubble 6px → 12px1397bbc8d6honor\nin question text viawhite-space: pre-wrap5d88316132question option cards 6px → 20px (larger cards need proportionally larger radius for equivalent visual softness)Test plan
Manually verified in worktree Electron dev at md-range window (
bun run dev:desktop):\nline breaksRelated
Summary by CodeRabbit
Style
Refactor
Chores
Tests