Skip to content

fix(web): review summary only says optional when it is - #18451

Merged
maria-rcks merged 1 commit into
pingdotgg:mainfrom
flamboh:fix/pr-review-form-honest-summary
Oct 14, 2026
Merged

maria-rcks merged 1 commit into
pingdotgg:mainfrom
flamboh:fix/pr-review-form-honest-summary

Conversation

@flamboh

@flamboh flamboh commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Note

🤖 Claude Opus 5.5 on behalf of Oliver

Problem

The pull request review composer always labeled the summary "(optional)", but a Comment review needs a summary or at least one line comment. The Submit button respected that rule, so on a review with no line comments the form said the summary was optional and then wouldn't submit. This shows up most on your own pull request. GitHub and Forgejo only let a PR's author leave Comment reviews, so the verdict picker there also had just one option, which looked broken.

Fix

  • The placeholder is computed from the same check as the Submit button, so they can't disagree. It says "(optional)" only when an empty summary would actually submit (Approve, or a review that already has line comments). Otherwise it says "Summarize your review or add line comments", or keeps the Forgejo "(required to request changes)" text when that verdict is selected.
  • The verdict picker is hidden when only one verdict is available, on both web and mobile. Mobile's segmented control had the same single-option problem; its placeholder never claimed the summary was optional.

Evidence

Review tab, no line comments, against a GitHub sandbox repo.

Your own PR (Comment is the only verdict)

Before After
Before: own PR, the summary says optional while Submit is disabled, and the verdict picker has one option After: own PR, the placeholder asks for a summary or line comments and the single-option picker is gone

Someone else's PR (all three verdicts)

Before After
Before: another author's PR, the summary is labeled optional with Comment selected After: another author's PR, the verdict picker is still shown and the placeholder states what is required

Mobile, your own PR (iOS simulator)

Before After
Before: mobile review sheet on your own PR shows a segmented control with only Comment After: the one-option segmented control is gone

With the after build, submitting a Comment review with a summary from the simulator still works: GitHub recorded it as COMMENTED on the sandbox pull request.

Validation

  • vp lint and vp fmt on both files
  • tsc --noEmit for apps/web and apps/mobile
  • Before/after captures from a worktree dev server, web and iOS simulator
  • Submitted a Comment review from the iOS simulator on your own pull request

Made with Claude Opus 5.5 in Claude Code, running in T3 Code.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. size:M 30-99 changed lines (additions + deletions). labels Oct 12, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 12, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 12, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at cbc0713

Macroscope's review found this PR approvable — This is a contained pull-request review UI fix: the web placeholder now matches existing submit requirements, and redundant single-option verdict controls are hidden on web and mobile. No schema, infrastructure, security-sensitive, or static-analysis changes are involved.

Notes:

  • Diff unchanged. Approvability was decided on eligibility alone.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: c7dde6ea-e8d5-4491-8e1d-ac5acd6e9f68



📥 Commits

Reviewing files that changed from the base of the PR and between d45d1a1 and cbc0713.




📒 Files selected for processing (2)
  • apps/mobile/src/features/pull-requests/PullRequestReviewSheet.tsx
  • apps/web/src/components/pullRequest/PullRequestReviewForm.tsx



Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.





📝 Walkthrough
📝 Walkthrough
📝 Walkthrough

Walkthrough

The mobile and web review forms now hide the verdict selector when one or fewer verdicts are offered. The web form updates submission eligibility checks and summary prompts based on the selected verdict and available review content.

Changes

Review form behavior

Layer / File(s) Summary
Web submission eligibility and prompts
apps/web/src/components/pullRequest/PullRequestReviewForm.tsx
Submission eligibility now checks the selected verdict and supplied summary. Request-changes reviews require a summary; other verdicts can submit when approving or when a summary or inline comments are present. The placeholder reflects the selected verdict and eligibility.
Verdict selector visibility and layout
apps/mobile/src/features/pull-requests/PullRequestReviewSheet.tsx, apps/web/src/components/pullRequest/PullRequestReviewForm.tsx
Both forms show the verdict selector only when multiple verdicts are offered. The web form positions the selector before the submit button.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: juliusmarminge





Merge Risk: ⚪ Minimal · up to cbc07

No merge-blocking issue is established; the mobile form can still submit a review when its only verdict is hidden.

Pre-merge checks | Passed 3 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check Warning The description explains the problem, fix, evidence, validation, and agent attribution. It does not include the required Scope and approval section or explain why this focused fix qualifies without an… Add the Scope and approval section. Link the triaged issue or explicit maintainer approval, or explain why this obvious bug fix qualifies for the exemption.
✅ Passed checks (3 passed)
Check name Status Explanation
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Title check Passed The title clearly and concisely describes the main fix: correcting the review summary’s optional label.

Full details: Description check

Explanation

The description explains the problem, fix, evidence, validation, and agent attribution. It does not include the required Scope and approval section or explain why this focused fix qualifies without an issue or maintainer approval.


  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR





  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

Copy link
Copy Markdown
Member

Note

This comment is posted by Julius' dot

The web before/after images show the corrected placeholder and verdict picker. The mobile picker also changes, but the PR says mobile was only typechecked. Please add mobile before/after screenshots showing the single-verdict case and confirm that submitting a Comment review still works, as required by the verification guide.

@flamboh

flamboh commented Oct 12, 2026

Copy link
Copy Markdown
Contributor Author

Note

🤖 Claude Opus 5.5 on behalf of Oliver

Added mobile before/after to the description, from the iOS simulator on a pull request the viewer authored, where Comment is the only verdict. The one-option segmented control is gone after the change.

Submitting still works: with the after build, I submitted a Comment review with a summary from the simulator, and GitHub recorded it as COMMENTED on the sandbox pull request.

@flamboh
flamboh marked this pull request as ready for review October 12, 2026 02:32
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 12, 2026 02:32

Dismissing prior approval to re-evaluate cbc0713

@juliusmarminge juliusmarminge added the macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews label Oct 12, 2026 — with ChatGPT Codex Connector
@maria-rcks
maria-rcks merged commit 7897a69 into pingdotgg:main Oct 14, 2026
45 checks passed
github-actions Bot added a commit to omarcresp/t3code-flake that referenced this pull request Oct 15, 2026
## What's Changed
* fix(server): only Claude progress updates before a tool call show as text by @maria-rcks in pingdotgg/t3code#18923
* fix(pr): keep known state in sync across controls by @maria-rcks in pingdotgg/t3code#18408
* fix(web): review summary only says optional when it is by @flamboh in pingdotgg/t3code#18451
* fix(web): subagent lists in the composer banner no longer start lines with a comma by @flamboh in pingdotgg/t3code#19170
* fix(web,mobile): pull request filters no longer offer the Scratch folder by @flamboh in pingdotgg/t3code#19179
* fix(web,mobile): pull request project filter follows the sidebar order by @flamboh in pingdotgg/t3code#19180
* feat(settings): inactivity auto-settle can keep threads with a PR by @tris203 in pingdotgg/t3code#14642
* fix(server): Claude works next to an enterprise managed-mcp.json by @SunkenInTime in pingdotgg/t3code#18706
* fix(web,mobile): muse code uses meta's official blue logo by @maria-rcks in pingdotgg/t3code#19562
* fix(desktop): import CA trust variables from the login shell by @maria-rcks in pingdotgg/t3code#19604
* fix(server): preview copy only forwards text from the viewer's copy keys by @maria-rcks in pingdotgg/t3code#19561
* fix(muse): Supervised threads ask before commands again by @maria-rcks in pingdotgg/t3code#19558
* fix(server): restarts no longer stall on old subagent result deliveries by @t3dotgg in pingdotgg/t3code#19631
* fix(clients): a thread comes back when its subagent needs approval by @t3dotgg in pingdotgg/t3code#19630
* fix(usage): cold usage scans no longer stall the server by @t3dotgg in pingdotgg/t3code#19446
* fix(server): "Always allow this session" stops Codex asking for every command by @t3dotgg in pingdotgg/t3code#19635
* fix(web): keep vertical cursor stops outside styled markers by @Yash-Singh1 in pingdotgg/t3code#19651
* fix(web): mermaid code toggle stays in place when switching views by @maria-rcks in pingdotgg/t3code#19601


**Full Changelog**: pingdotgg/t3code@v0.0.46-nightly.20261014.3118...v0.0.46-nightly.20261014.3132

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.46-nightly.20261014.3132
github-actions Bot added a commit to davidvanderklay/t3code-flake that referenced this pull request Oct 15, 2026
## What's Changed
* fix(server): only Claude progress updates before a tool call show as text by @maria-rcks in pingdotgg/t3code#18923
* fix(pr): keep known state in sync across controls by @maria-rcks in pingdotgg/t3code#18408
* fix(web): review summary only says optional when it is by @flamboh in pingdotgg/t3code#18451
* fix(web): subagent lists in the composer banner no longer start lines with a comma by @flamboh in pingdotgg/t3code#19170
* fix(web,mobile): pull request filters no longer offer the Scratch folder by @flamboh in pingdotgg/t3code#19179
* fix(web,mobile): pull request project filter follows the sidebar order by @flamboh in pingdotgg/t3code#19180
* feat(settings): inactivity auto-settle can keep threads with a PR by @tris203 in pingdotgg/t3code#14642
* fix(server): Claude works next to an enterprise managed-mcp.json by @SunkenInTime in pingdotgg/t3code#18706
* fix(web,mobile): muse code uses meta's official blue logo by @maria-rcks in pingdotgg/t3code#19562
* fix(desktop): import CA trust variables from the login shell by @maria-rcks in pingdotgg/t3code#19604
* fix(server): preview copy only forwards text from the viewer's copy keys by @maria-rcks in pingdotgg/t3code#19561
* fix(muse): Supervised threads ask before commands again by @maria-rcks in pingdotgg/t3code#19558
* fix(server): restarts no longer stall on old subagent result deliveries by @t3dotgg in pingdotgg/t3code#19631
* fix(clients): a thread comes back when its subagent needs approval by @t3dotgg in pingdotgg/t3code#19630
* fix(usage): cold usage scans no longer stall the server by @t3dotgg in pingdotgg/t3code#19446
* fix(server): "Always allow this session" stops Codex asking for every command by @t3dotgg in pingdotgg/t3code#19635
* fix(web): keep vertical cursor stops outside styled markers by @Yash-Singh1 in pingdotgg/t3code#19651
* fix(web): mermaid code toggle stays in place when switching views by @maria-rcks in pingdotgg/t3code#19601


**Full Changelog**: pingdotgg/t3code@v0.0.46-nightly.20261014.3118...v0.0.46-nightly.20261014.3132

Upstream release: https://github.com/pingdotgg/t3code/releases/tag/v0.0.46-nightly.20261014.3132
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

macroscope-review Opt PRs made by unvouched contributors in for Macroscope review. Vouched contributors auto-reviews size:M 30-99 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants