[akhil9tiet]

Fixes #2224

The second call overwrites the constraint's index and the list holds it twice, so the problem has an inconsistent constraint list. This change adds a guard in addConstraint that throws IllegalArgumentException when the constraint is already in the list, checked before any rename/reindex so a rejected duplicate leaves the list untouched. Constraint has no equals override so the check is identity-based.

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: NVIDIA/cuopt
• Branch: main
• File: .coderabbit.yaml
• Commit: 4ed7f37
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Enterprise
• Run ID: f0190f91-133d-4011-8ebc-837c61541bb3

Commits

Reviewing files that changed from the base of the PR and between 4ed7f37 and dcf4725.

Files selected for processing: 1

• java/cuopt/src/main/java/com/nvidia/cuopt/mathematicaloptimization/Problem.java

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

Walkthrough

Problem.addConstraint now rejects a constraint that is already present in the problem. It throws IllegalArgumentException before ownership validation or changes to the constraint’s name and index.

Changes: Duplicate constraint rejection

Layer / File(s)	Summary
Reject an already-added constraint java/cuopt/src/main/java/com/nvidia/cuopt/mathematicaloptimization/Problem.java	addConstraint throws IllegalArgumentException when the constraint is already in the problem. The check runs before ownership validation or changes to the constraint’s name and index.

Priority: Low

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

Change: Bug fix · Severity of issue fixed: Low

Possibly related PRs

• NVIDIA/cuopt#2226: This PR rejects the same constraint object on a second call to Problem.addConstraint.
• NVIDIA/cuopt#2219: This PR adds a test for adding the same constraint twice.

Suggested reviewers: iroy30

Merge Risk: Minimal · up to dcf47

The duplicate-constraint behavior matches the requested contract, with no identified merge-blocking risk.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly and concisely describes the primary change: rejecting duplicate additions of the same Constraint to a Problem.
Description check		The description directly explains the duplicate-constraint defect and the guard added to preserve the problem state.
Linked Issues check		Issue #2224 requires Problem.addConstraint to reject the same Constraint object twice with IllegalArgumentException and keep the constraint count at one. The change checks `constraints.contains(…
Out of Scope Changes check		The whole pull request changes only Problem.addConstraint and adds the duplicate-constraint guard required by #2224. No unrelated production changes are present.

Full details: Linked Issues check

Explanation

Issue #2224 requires Problem.addConstraint to reject the same Constraint object twice with IllegalArgumentException and keep the constraint count at one. The change checks constraints.contains(constraint) before ownership validation, renaming, and reindexing. Constraint has no equals override, so the check is identity-based. The rejected call does not modify the list.

Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 1 functions across 1 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1

🛠️ Fix failing CI checks: 💡

• Commit to this branch
• Create a new PR

🧪 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.}

[ramakrishnap-nv]

This duplicates #2226, which makes the same change for #2224. Could you close this one and continue on #2226? Either way, please add a test that adding the same Constraint twice throws IllegalArgumentException and leaves getNumConstraints() at 1.

[akhil9tiet]

Closing per maintainer request — continuing on #2226, which now carries the test and the formatting fix.