Skip to content

review: judge UX fit and placement, not only code - #74

Open
parsakhaz wants to merge 1 commit into
mainfrom
feat/review-ux-fit
Open

review: judge UX fit and placement, not only code#74
parsakhaz wants to merge 1 commit into
mainfrom
feat/review-ux-fit

Conversation

@parsakhaz

Copy link
Copy Markdown
Member

What

A new section 6, UX Fit and Placement (Should-Fix), in review/CRITERIA.md, and the section numbering and severity mapping in review/SKILL.md updated to match.

For any change that adds or moves user-facing surface, the reviewer asks these before the code questions:

  • placement matches the information's scope (account-level in Settings, per-item beside the item, global actions in a global bar)
  • progressive disclosure: appears when it can show something, hidden otherwise; detection beats a toggle
  • scoped to the panels it concerns
  • one way to reach it
  • fits the surface it joins, and joins an existing home before creating a rival one
  • comparable products: where do mature tools put this?

The reviewer recommends the placement, names why, gives the path, and says when the plumbing underneath is right and reusable.

Why

Pane #398 (Codex usage widget): a toolbar toggle that showed on every panel type including plain terminals, enabled via a hidden toggle plus a persisted preference plus an auto-open, putting account-level usage in a per-pane sidebar. It went through the review, the refactor pass, and three adversary rounds, all judging correctness, and every one passed the placement. Parsa found it in thirty seconds of using the app. The fix is a Settings → Usage tab shown when a Codex login is detected. That question belongs at review, before hardening starts.

Should-Fix tier: a misplaced surface merges cleanly and still costs every user.

🤖 Generated with Claude Code

Any change that adds or moves user-facing surface gets judged on
placement before code: scope-matched placement (account-level in
Settings), progressive disclosure (appears when it can show something;
detection over toggles), scoped to the panels it concerns, one way to
reach it, fits the surface it joins, comparable products.

Surfaced by Pane #398: three rounds of correctness hardening on a
widget whose toolbar toggle showed on every panel type and whose
account-level data lived in a per-pane sidebar. Every review judged
the code; none asked where the feature belonged.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 264c11a48f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

- No `I` prefix on interface names

## 6. Tailwind CSS and shadcn/ui (Suggestion)
## 6. UX Fit and Placement (Should-Fix)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Update the implementation-reviewer severity mapping

When the implementation-reviewer reviews a UI change, .claude/agents/implementation-reviewer.md:71-73 still labels sections 6-7 as suggestions. The newly inserted section 6 is therefore downgraded from Should-Fix to low priority, and the renumbered conventions section 8 is absent from its focus map. Update that consumer to use sections 3-6 for Should-Fix and 7-8 for suggestions, as this PR review skill now does.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant