Skip to content

ARR: Add comment preprocess for links#2976

Open
haroldrubio wants to merge 10 commits intomasterfrom
fix/arr-comment-preprocess
Open

ARR: Add comment preprocess for links#2976
haroldrubio wants to merge 10 commits intomasterfrom
fix/arr-comment-preprocess

Conversation

@haroldrubio
Copy link
Copy Markdown
Member

@haroldrubio haroldrubio commented Mar 26, 2026

This PR adds a pre-process for the ARR official comment to prevent links being sent to and from authors. The check is just for http/https as requested in the feature request

@haroldrubio haroldrubio self-assigned this Mar 26, 2026
@melisabok melisabok added the arr Issues related to the ARR workflow label Apr 8, 2026
@haroldrubio haroldrubio marked this pull request as ready for review April 9, 2026 18:06
Copilot AI review requested due to automatic review settings April 9, 2026 18:06
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds an ARR-specific preprocess script for Official_Comment notes to block link-containing comments when they are visible to authors, and updates the ARR venue configuration and tests accordingly.

Changes:

  • Add openreview/arr/process/comment_pre_process.js to enforce “no links” for author-visible official comments (while preserving mandatory-reader checks).
  • Configure ARR’s comment_stage to use the new preprocess script.
  • Extend test_author_response to assert link blocking for author-visible official comments and allow links for non-author-visible internal comments.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/test_arr_venue_v2.py Adds regression coverage for link-blocking behavior in Official_Comment when authors are readers.
openreview/arr/process/comment_pre_process.js Implements ARR-specific preprocess logic to detect links and reject author-visible official comments containing them.
openreview/arr/arr.py Wires the ARR comment stage to use the new preprocess script.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread openreview/arr/process/comment_pre_process.js Outdated
Comment thread tests/test_arr_venue_v2.py
const commentText = note.content?.comment?.value || ""
const linkPattern = /https?:\/\//i

if ((readers.includes(authorGroupId) || readers.includes("everyone")) && linkPattern.test(commentText)) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

have you tried setting a regex in the comment field of the Official_Comment invitation?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The regex would apply to all comments, so this would disallow links in comments between reviewers/ACs/SACs right? The request is "authors could be prevented from submitting responses to reviewers" which I think needs extra logic

const authorsName = domain.content?.authors_name?.value || "Authors"
const authorGroupId = `${domain.id}/Submission${forum.number}/${authorsName}`
const commentText = note.content?.comment?.value || ""
const linkPattern = /https?:\/\//i
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We still need to allow links to the openreview site.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I still don't think this validation is good enough, anyone can add a link without https?, what is the purpose of this validation?

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

Labels

arr Issues related to the ARR workflow

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants