Skip to content

Add venue profile validation for invite assignment#2572

Open
enrubio wants to merge 18 commits intomasterfrom
feature/add-venue-profile-check
Open

Add venue profile validation for invite assignment#2572
enrubio wants to merge 18 commits intomasterfrom
feature/add-venue-profile-check

Conversation

@enrubio
Copy link
Copy Markdown
Member

@enrubio enrubio commented Apr 24, 2025

This PR adds a new setting for the venue group content (profile_minimum_requirements) that allows venues to define profile requirements when inviting reviewers. Validation is added to the check_new_profiles cron job and to the invite assignment preprocess. Example of setting:

'profile_minimum_requirements': { 
    'value': {
        'relations': true,
        'history': true,
        'expertise': true,
        'publications': true,
        'active': true
    } 
}

If a user was invited, is in Pending Sign Up, and still has an incomplete profile, that user will stay in Pending Sign Up and receive email reminders each day until they complete their profile. Then their profile is checked for conflicts.

The email they receive will list what requirements they are missing.

Changes need to match this PR in openreview-js, so before merging this we should make sure there's not more feedback on that PR that needs to be moved here.

Comment thread openreview/venue/process/invite_assignment_pre_process.js Outdated
Comment thread openreview/venue/process/invite_assignment_pre_process.js Outdated
@enrubio enrubio marked this pull request as ready for review May 6, 2025 13:31
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

This PR adds and enforces a new setting (invited_reviewer_profile_minimum_requirements) to validate reviewer profiles before invitation assignment, ensuring that only profiles meeting venue‐specific minimum criteria are accepted.

  • Updated tests in test_cvpr_conference_v2.py to verify profile validation logic in invite assignments.
  • Modified openreview/venue/venue.py and openreview/venue/process/invite_assignment_pre_process.js to incorporate profile completeness checks and send notifications accordingly.
  • Propagated the new setting into group and conference helper files to support its configuration and use.

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
tests/test_cvpr_conference_v2.py Added new test for invite assignment profile requirements and validations.
openreview/venue/venue.py Updated check_new_profiles to validate profiles based on new requirements and send notifications.
openreview/venue/process/invite_assignment_pre_process.js Integrated profile completeness check in invite assignment preprocess.
openreview/venue/group.py Propagated the new invited_reviewer_profile_minimum_requirements setting into group content.
openreview/conference/helpers.py Updated venue configuration to load the new profile requirements setting.
Comments suppressed due to low confidence (1)

tests/test_cvpr_conference_v2.py:1524

  • Consider replacing the use of time.sleep(5) with a more robust waiting mechanism (e.g., using helpers.await_queue_edit) to reliably wait for asynchronous updates and reduce test flakiness.
time.sleep(5)

Comment thread openreview/venue/venue.py Outdated
Comment thread openreview/venue/process/invite_assignment_pre_process.js Outdated
Comment thread openreview/venue/venue.py
Comment thread openreview/venue/venue.py Outdated
@melisabok
Copy link
Copy Markdown
Member

@enrubio I would like to make progress on this, I think this is going to be asked by future venues, please let me know if you need help to finish it.

@melisabok
Copy link
Copy Markdown
Member

Any updates on this @enrubio ?

If not, let's close it.

@enrubio
Copy link
Copy Markdown
Member Author

enrubio commented Nov 6, 2025

This is on my todo list. I was going to work on this after I work on another older PR, but I can fix this one next.

We discussed simplifying this so that venues can only require specific fields in the profile for invite assignment (e.g. requiring at least 1 Relations for conflict checking) instead of allowing them to require any profile field. I think that's much less complicated than the original changes.

@enrubio
Copy link
Copy Markdown
Member Author

enrubio commented Mar 10, 2026

I simplified the logic so that PCs should only require: history, relations, expertise, publications, and active state. Previously they could require any part of the profile, but that includes links which we should not allow.

Tests will fail because it relies on openreview-js changes.

@enrubio
Copy link
Copy Markdown
Member Author

enrubio commented Apr 9, 2026

@melisabok @celestemartinez can you check this when you can?

If the logic in check_profile_minimum_requirements looks good, then the JS PR can be merged and the circle CI tests here can be re-run.

}

// Check for complete profile, if no profile then go to pending sign up
if (profileReqs && !client.tools.checkProfileMinimumRequirements(userProfile, profileReqs)) {
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.

do these requirements apply to all the committee role or just reviewers?

if venue.use_publication_chairs:
preferred_emails_groups.append(venue.get_publication_chairs_id())
venue.preferred_emails_groups = preferred_emails_groups
venue.profile_minimum_requirements = venue_content.get('profile_minimum_requirements', {}).get('value', False)
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 would prefer a more descriptive name and instead of storing this in the venue domain, we can store it in the invite assignment invitation so it can be different per committee role.

What about calling it "invitee_profile_minim_requirement"?

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 wish the new UI can have a subinvitation to edit the Invite_Assignment invitation to configure these requirements.

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

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

Comments suppressed due to low confidence (1)

openreview/venue/venue.py:2414

  • The profiles[0].active gate prevents processing profiles that exist but are not active yet. If a venue sets profile_minimum_requirements.active=true, this code will never send the “profile not active” reminder because it exits before running the requirement checks. Consider removing the .active guard and letting check_profile_minimum_requirements (and the notification text) handle the active requirement explicitly.
                            profiles=openreview.tools.get_profiles(venue_client, [tail], with_publications=True, with_relations=True)

                            if profiles and profiles[0].active:

                                user_profile=profiles[0]

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

const conflictPolicy = domain.content?.[`${committeeRole}_conflict_policy`]?.value
const conflictNYears = domain.content?.[`${committeeRole}_conflict_n_years`]?.value
const quota = domain.content?.[`submission_assignment_max_${committeeRole}`]?.value
const profileReqs = domain.content.profile_minimum_requirements?.value
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

domain.content is accessed without optional chaining when reading profile_minimum_requirements. If domain.content is undefined/null (which other lines already guard against with domain.content?.[...]), this will throw and break the invite pre-process. Use domain.content?.profile_minimum_requirements?.value (and similarly guard any nested access) to keep behavior consistent with the rest of the file.

Suggested change
const profileReqs = domain.content.profile_minimum_requirements?.value
const profileReqs = domain.content?.profile_minimum_requirements?.value

Copilot uses AI. Check for mistakes.
Comment thread openreview/venue/venue.py
if conflicts:
print(f'Conflicts detected for {edge.head} and {user_profile.id}', conflicts)
mark_as_conflict(venue_group, edge, submission, user_profile)
if is_incomplete:
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

When a real profile is found for a pending-signup edge and the profile is still incomplete, the code only sends a notification but does not persist any edge update. This means the edge tail stays as the raw email until the profile becomes complete, which conflicts with the intended behavior in the added test (email-tail edge removed, profile-id-tail edge remains in Pending Sign Up) and keeps re-grouping by email. Consider updating/posting the edge to set edge.tail = user_profile.id (keeping label Pending Sign Up) before/while sending the incomplete-profile notification.

Suggested change
if is_incomplete:
if is_incomplete:
edge.tail = user_profile.id
venue_client.post_edge(edge)

Copilot uses AI. Check for mistakes.
Comment thread openreview/tools.py
if not profile.content.get(field):
return False
elif field == 'active':
if not profile.state or 'active' not in profile.state.lower():
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

check_profile_minimum_requirements checks the active requirement via profile.state, but other code paths (e.g., check_new_profiles) treat profile.active as the authoritative boolean. If state is missing while active is true, this will incorrectly flag profiles as incomplete. Consider checking profile.active (or falling back to state only when active is None) for the active requirement.

Suggested change
if not profile.state or 'active' not in profile.state.lower():
profile_active = getattr(profile, 'active', None)
if profile_active is not None:
if not profile_active:
return False
elif not profile.state or 'active' not in profile.state.lower():

Copilot uses AI. Check for mistakes.
if venue.use_publication_chairs:
preferred_emails_groups.append(venue.get_publication_chairs_id())
venue.preferred_emails_groups = preferred_emails_groups
venue.profile_minimum_requirements = venue_content.get('profile_minimum_requirements', {}).get('value', False)
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

venue.profile_minimum_requirements is set to False when absent, but elsewhere it is initialized as {} and treated like a dict of requirements. Using {}/None here would keep the type consistent and avoid callers needing to handle both dict and bool values.

Suggested change
venue.profile_minimum_requirements = venue_content.get('profile_minimum_requirements', {}).get('value', False)
venue.profile_minimum_requirements = venue_content.get('profile_minimum_requirements', {}).get('value', {})

Copilot uses AI. Check for mistakes.
Comment on lines +1685 to +1688
time.sleep(5)

## Check new message sent because of missing publication
messages = openreview_client.get_messages(to='celeste@acm.org', subject='[CVPR 2024] Incomplete profile for paper 1')
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

The added time.sleep(5) calls can make this test suite slow and flaky (timing-dependent), especially in CI. Prefer waiting on the relevant queue edit(s) or polling get_messages(...) until the expected count appears (with a timeout) instead of a fixed sleep.

Suggested change
time.sleep(5)
## Check new message sent because of missing publication
messages = openreview_client.get_messages(to='celeste@acm.org', subject='[CVPR 2024] Incomplete profile for paper 1')
## Check new message sent because of missing publication
deadline = time.time() + 30
messages = []
while time.time() < deadline:
messages = openreview_client.get_messages(
to='celeste@acm.org',
subject='[CVPR 2024] Incomplete profile for paper 1'
)
if messages and len(messages) == 3:
break
time.sleep(0.5)

Copilot uses AI. Check for mistakes.

openreview.venue.Venue.check_new_profiles(openreview_client)

time.sleep(5)
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

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

Another fixed time.sleep(5) is used here to wait for side effects. To reduce flakiness and avoid unnecessary delays, prefer awaiting the relevant queue edits or polling get_messages(...) with a timeout instead of sleeping a fixed duration.

Suggested change
time.sleep(5)
deadline = time.time() + 10
while True:
conflict_messages = openreview_client.get_messages(
to='celeste@acm.org',
subject='[CVPR 2024] Conflict detected for paper 1'
)
invite_edges = pc_client.get_edges(
invitation='thecvf.com/CVPR/2024/Conference/Reviewers/-/Invite_Assignment',
head=submissions[0].id,
tail='~Celeste_ACM1'
)
if (
conflict_messages and len(conflict_messages) == 1 and
len(invite_edges) == 1 and invite_edges[0].label == 'Conflict Detected'
):
break
if time.time() >= deadline:
break
time.sleep(0.5)

Copilot uses AI. Check for mistakes.
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.

3 participants