Skip to content

🐛 Fixed reply form showing parent comment author's details#29

Open
tomerqodo wants to merge 2 commits intoqodo_action_req_base__fixed_reply_form_showing_parent_comment_authors_details_pr2from
qodo_action_req_head__fixed_reply_form_showing_parent_comment_authors_details_pr2
Open

🐛 Fixed reply form showing parent comment author's details#29
tomerqodo wants to merge 2 commits intoqodo_action_req_base__fixed_reply_form_showing_parent_comment_authors_details_pr2from
qodo_action_req_head__fixed_reply_form_showing_parent_comment_authors_details_pr2

Conversation

@tomerqodo
Copy link
Copy Markdown

Benchmark PR from agentic-review-benchmarks#2

rob-ghost and others added 2 commits January 25, 2026 11:56
ref https://linear.app/ghost/issue/BER-3212

When a user opened a reply form, the form header was showing the parent
comment author's name and expertise instead of the current user's data.
This happened because the code used a fallback pattern that pulled from
the comment being replied to when the current member had no value set.
@qodo-code-review
Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (2) 📘 Rule violations (1) 📎 Requirement gaps (0)

Grey Divider


Action required

1. openForm undefined access 📘 Rule violation ⛯ Reliability
Description
FormWrapperProps declares openForm as optional, but the new code dereferences it with
  openForm.in_reply_to_snippet.
• If isOpen is true while openForm is undefined, this will throw at runtime and break the
  reply form UI instead of degrading gracefully.
Code

apps/comments-ui/src/components/content/forms/form.tsx[313]

+        const isReplyToReply = !!openForm.in_reply_to_snippet;
Evidence
PR Compliance ID 3 requires explicit handling of null/undefined edge cases. The updated code removes
the null-safe access (?.) while openForm remains optional in the props type, creating a
potential runtime failure when openForm is absent.

Rule 3: Generic: Robust Error Handling and Edge Case Management
apps/comments-ui/src/components/content/forms/form.tsx[291-315]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
`openForm` is typed as optional in `FormWrapperProps`, but the code dereferences it with `openForm.in_reply_to_snippet` after removing optional chaining. This can throw when `openForm` is `undefined`.

## Issue Context
This violates the robustness/edge-case handling requirement because it introduces a potential null/undefined runtime failure.

## Fix Focus Areas
- apps/comments-ui/src/components/content/forms/form.tsx[291-315]સ્ત

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Reply-to-reply broken 🐞 Bug ✓ Correctness
Description
• ReplyForm now always sends in_reply_to_id: parent.id, which breaks reply-to-reply behavior.
• For reply-to-reply, the UI constructs openForm.in_reply_to_id as the specific reply being
  replied to; losing that changes threading semantics and can break “Reply to …” context/snippet
  handling.
Code

apps/comments-ui/src/components/content/forms/reply-form.tsx[R28-33]

            reply: {
                post_id: postId,
-                in_reply_to_id: openForm.in_reply_to_id,
+                in_reply_to_id: parent.id,
                status: 'published',
                html
            }
Evidence
When replying to a reply, openReplyForm sets OpenCommentForm.in_reply_to_id to the reply’s id
and separately sets parent_id to the top-level comment. ReplyForm is rendered under the
top-level comment (passed as parent), so using parent.id for in_reply_to_id is incorrect for
nested replies.

apps/comments-ui/src/components/content/comment.tsx[107-120]
apps/comments-ui/src/components/content/comment.tsx[244-252]
apps/comments-ui/src/app-context.ts[34-41]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`ReplyForm` now always sends `in_reply_to_id: parent.id`, which is wrong for reply-to-reply flows. For nested replies, `openForm.in_reply_to_id` is intentionally set to the specific reply being replied to.

### Issue Context
`openReplyForm` constructs `OpenCommentForm` with `parent_id` (top-level thread) and `in_reply_to_id`/`in_reply_to_snippet` (the specific comment being replied to).

### Fix Focus Areas
- apps/comments-ui/src/components/content/forms/reply-form.tsx[24-36]
- apps/comments-ui/src/components/content/comment.tsx[103-124]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Remediation recommended

3. Editor gated by expertise 🐞 Bug ✓ Correctness
Description
• Form now disables editing unless the member has expertise, even though expertise is treated as
  optional elsewhere.
• This likely prevents users with a valid name but no expertise from typing comments/replies, and
  contradicts both the inline comment and the profile popup validation (which only requires a name).
Code

apps/comments-ui/src/components/content/forms/form.tsx[R263-265]

        // Disable editing if the member doesn't have a name or when we are submitting the form
-        editor.setEditable(!!memberName && progress !== 'sending');
-    }, [editor, memberName, progress]);
+        editor.setEditable(!!member?.expertise && progress !== 'sending');
+    }, [editor, member, progress]);
Evidence
The form logic and profile modal enforce/require only a non-empty name; expertise can be empty. The
new setEditable condition uses expertise instead of name, which conflicts with the rest of the
flow (focus logic forces name first; popup submission only blocks on empty name).

apps/comments-ui/src/components/content/forms/form.tsx[258-265]
apps/comments-ui/src/components/content/forms/form.tsx[355-362]
apps/comments-ui/src/components/popups/add-details-popup.tsx[38-50]
apps/comments-ui/src/actions.ts[374-378]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
The editor is currently disabled unless the member has `expertise`, but existing UX and validation treat expertise as optional and only require a name. This can prevent users (with a name but no expertise) from typing.

### Issue Context
- FormWrapper focus logic forces name collection only.
- Add-details popup blocks submission only when name is empty.

### Fix Focus Areas
- apps/comments-ui/src/components/content/forms/form.tsx[258-265]
- apps/comments-ui/src/components/content/forms/form.tsx[355-362]
- apps/comments-ui/src/components/popups/add-details-popup.tsx[38-50]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



ⓘ The new review experience is currently in Beta. Learn more

Qodo Logo

let openStyles = '';
if (isOpen) {
const isReplyToReply = !!openForm?.in_reply_to_snippet;
const isReplyToReply = !!openForm.in_reply_to_snippet;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

1. openform undefined access 📘 Rule violation ⛯ Reliability

FormWrapperProps declares openForm as optional, but the new code dereferences it with
  openForm.in_reply_to_snippet.
• If isOpen is true while openForm is undefined, this will throw at runtime and break the
  reply form UI instead of degrading gracefully.
Agent prompt
## Issue description
`openForm` is typed as optional in `FormWrapperProps`, but the code dereferences it with `openForm.in_reply_to_snippet` after removing optional chaining. This can throw when `openForm` is `undefined`.

## Issue Context
This violates the robustness/edge-case handling requirement because it introduces a potential null/undefined runtime failure.

## Fix Focus Areas
- apps/comments-ui/src/components/content/forms/form.tsx[291-315]સ્ત

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Comment on lines 28 to 33
reply: {
post_id: postId,
in_reply_to_id: openForm.in_reply_to_id,
in_reply_to_id: parent.id,
status: 'published',
html
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Action required

2. Reply-to-reply broken 🐞 Bug ✓ Correctness

• ReplyForm now always sends in_reply_to_id: parent.id, which breaks reply-to-reply behavior.
• For reply-to-reply, the UI constructs openForm.in_reply_to_id as the specific reply being
  replied to; losing that changes threading semantics and can break “Reply to …” context/snippet
  handling.
Agent prompt
### Issue description
`ReplyForm` now always sends `in_reply_to_id: parent.id`, which is wrong for reply-to-reply flows. For nested replies, `openForm.in_reply_to_id` is intentionally set to the specific reply being replied to.

### Issue Context
`openReplyForm` constructs `OpenCommentForm` with `parent_id` (top-level thread) and `in_reply_to_id`/`in_reply_to_snippet` (the specific comment being replied to).

### Fix Focus Areas
- apps/comments-ui/src/components/content/forms/reply-form.tsx[24-36]
- apps/comments-ui/src/components/content/comment.tsx[103-124]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

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.

2 participants