Repository navigation
fix(subagents): align ui designer preview verification - #301
Merged
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
BrowserPreview wording and mirrored locale/spec descriptions still overstate delegate-side verification capabilities.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
This PR aligns the ui-designer subagent guidance with BrowserPreview’s live-reload-only behavior.
Changes:
- Updates shared and runtime descriptions and verification instructions.
- Adds preset/runtime synchronization tests.
- Documents BrowserPreview limitations in the runtime specification.
File summaries
| File | Reviewed changes and findings |
|---|---|
packages/shared/src/subagent-presets.ts |
Updates the UI designer preset. Moderate (3 votes): wording still implies the delegate can inspect the preview. Nit (2 votes): eight localized descriptions and parity coverage still overstate browser-preview verification. |
packages/agent-runtime/src/subagent-definitions.ts |
Mirrors the runtime definition. Moderate (2 votes): instructions imply inspection unavailable from BrowserPreview. Moderate (1 vote): frontmatter description remains inconsistent with that boundary. |
packages/agent-runtime/src/subagent-definitions.test.ts |
Adds/verifies shared preset and runtime definition synchronization. |
docs/spec/03-runtime/02-agent-runtime.md |
Documents BrowserPreview limitations. Nit (3 votes): Chinese specification is not mirrored. Nit (1 vote): wording still claims the delegate can inspect the rendered result. |
Review details
Suppressed comments (2)
docs/spec/03-runtime/02-agent-runtime.md:710
- This sentence still says the delegate can inspect the rendered result, but the
BrowserPreviewresult only confirms that the work-panel opened; it exposes no screenshot, DOM, viewport, or interaction data to the delegate. The following sentence correctly assigns responsive and interaction checks to project tooling, so use wording such as "open the rendered result" here rather than claiming delegate inspection.
`BrowserPreview` so it can open and inspect its rendered result before reporting.
`BrowserPreview` only opens a live-reloading workspace HTML page; responsive,
keyboard-focus, and reduced-motion checks require project-provided browser
tests or other tooling. Its statuses are `completed`,
packages/agent-runtime/src/subagent-definitions.ts:157
- This description still says the result is "inspected in the browser preview", but the only result delivered to the delegate is the text confirmation from
BrowserPreview; no screenshot, viewport, or DOM state is available. Keep this frontmatter description aligned with the boundary documented below (the page is opened for preview, while actual verification requires project browser tooling), and update the matching shared preset at the same time.
description: Design and implement a web interface from a brief — visual system, motion and complete interaction states, inspected in the browser preview or project browser tests. Use for building or restyling a UI when the visual work should run in its own context.
- Files reviewed: 4/4 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+187
to
+191
| - Verify before reporting: after the first meaningful visual edit, call | ||
| BrowserPreview with a workspace-relative HTML path and inspect the live- | ||
| reloading page it opens. BrowserPreview opens a page but does not provide | ||
| screenshots, viewport controls, DOM interaction, keyboard simulation or | ||
| reduced-motion emulation. Use project-provided browser or E2E tooling through |
Comment on lines
+179
to
+186
| reloading page it opens. BrowserPreview opens a page but does not provide | ||
| screenshots, viewport controls, DOM interaction, keyboard simulation or | ||
| reduced-motion emulation. Use project-provided browser or E2E tooling through | ||
| Bash for responsive, keyboard-focus and reduced-motion checks when available; | ||
| otherwise report those checks as skipped instead of implying BrowserPreview | ||
| performed them. Fix what you observe and re-check. Run the project's build or | ||
| typecheck when it covers your change. A result you did not look at is not | ||
| evidence. |
Comment on lines
+707
to
+710
| `BrowserPreview` so it can open and inspect its rendered result before reporting. | ||
| `BrowserPreview` only opens a live-reloading workspace HTML page; responsive, | ||
| keyboard-focus, and reduced-motion checks require project-provided browser | ||
| tests or other tooling. Its statuses are `completed`, |
| name: "UI designer", | ||
| description: | ||
| "Design and implement a web interface from a brief — visual system, motion and complete interaction states, verified in the browser preview. Use for building or restyling a UI when the visual work should run in its own context.", | ||
| "Design and implement a web interface from a brief — visual system, motion and complete interaction states, inspected in the browser preview or project browser tests. Use for building or restyling a UI when the visual work should run in its own context.", |
This branch was successfully deployed
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
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.
Follow-up to #292.\n\nThe ui-designer builtin prompt now describes BrowserPreview accurately: it opens a workspace-relative, live-reloading HTML page but does not provide screenshots, viewport controls, DOM interaction, keyboard simulation, or reduced-motion emulation. The shared preset and runtime definition stay synchronized, and the runtime specification records the boundary.\n\nValidation:\n- @pi-desktop/shared tests: 249 passed\n- @pi-desktop/shared typecheck: passed\n- @pi-desktop/agent-runtime tests: 363 passed\n- @pi-desktop/agent-runtime typecheck: passed\n- host-core debug build: passed\n- subagents E2E: 19/19 passed