[DRAFT] refactor(tests): replace cuopt_static with CUOPT_INTERNAL_EXPORT + version script

[ramakrishnap-nv]

Builds on #1566. Eliminates cuopt_static from all test linkage by introducing CUOPT_INTERNAL_EXPORT for the small set of internal symbols that test TUs reference directly. A GNU ld version script (cmake/cuopt.map) assigns public symbols to CUOPT_1.0 and test-only internals to CUOPT_PRIVATE_1.0. Only generator::dataset_t and generate_dataset needed marking — all other routing internal types are template/inline and instantiate in-place. Removes ConfigureInternalTest entirely; all tests now use ConfigureTest (shared linkage).

[mlubin]

None of the C++ API is part of the stable ABI from what I understand. Only the C and Python interfaces are.

[ramakrishnap-nv]

Agree, I am currently trying a way to work on a way which would solve this problem of internal API usage as a whole. So just testing as of now.

[github-actions]

CI Test Summary

✅ All 31 test job(s) passed.

[coderabbitai]

📝 Walkthrough

Walkthrough

The change adds CUOPT visibility macros, applies them across public headers and explicit template instantiations, restructures CMake targets around an object library, adds dynamic-symbol validation, and runs that validation during the libcuopt package build.

Changes

Visibility, build, and symbol validation

Layer / File(s)	Summary
Export contracts cpp/include/cuopt/...	Adds visibility macros and applies them to public namespaces, C API declarations, and internal declarations.
Exported instantiations cpp/src/grpc/..., cpp/src/io/..., cpp/src/pdlp/..., cpp/src/routing/..., cpp/src/mip_heuristics/...	Marks explicit template instantiations with the appropriate export macro.
Object and test library wiring cpp/CMakeLists.txt, cpp/tests/...	Builds shared and static targets from common objects and updates test linkage selection.
Symbol checking and packaging ci/check_symbols.sh, conda/recipes/libcuopt/recipe.yaml	Checks exported symbols for disallowed patterns and runs the check during package creation.

Estimated code review effort: 4 (Complex) | ~60 minutes

Suggested reviewers: tmckayus, bubullzz, mlubin

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title matches the main change: moving tests off cuopt_static toward exported internals and a version script.
Description check	✅ Passed	The description directly describes the linkage refactor and export/visibility changes in the pull request.
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.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/internal-export-for-tests

---

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@ci/check_symbols.sh`:
- Around line 61-64: Update the non-trailing-scope matching logic in
has_trailing_scope so the pslp_ pattern matches symbols beginning with the plain
prefix pslp_, while preserving existing exact and ::-qualified matching behavior
for other patterns. Add a regression fixture covering a symbol such as pslp_foo.

In `@cpp/include/cuopt/export.hpp`:
- Line 8: Replace `#pragma` once with a unique `#ifndef/`#define include guard and
matching closing `#endif` in cpp/include/cuopt/export.hpp (lines 8-8) and
cpp/include/cuopt/routing/assignment.hpp (lines 8-8), ensuring each header uses
a distinct guard name.

In `@cpp/tests/linear_programming/grpc/CMakeLists.txt`:
- Line 27: Replace all three cuopt_static references in the gRPC test target
definitions with cuopt, ensuring every test links against the shared target and
validates its exported symbols.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: ceb19a9b-2b27-4132-bd57-a69cde59eec5

📥 Commits

Reviewing files that changed from the base of the PR and between cd8ddad and 8d7160b.

📒 Files selected for processing (73)

• ci/check_symbols.sh
• conda/recipes/libcuopt/recipe.yaml
• cpp/CMakeLists.txt
• cpp/include/cuopt/error.hpp
• cpp/include/cuopt/export.hpp
• cpp/include/cuopt/grpc/cython_grpc_client.hpp
• cpp/include/cuopt/grpc/grpc_client_env.hpp
• cpp/include/cuopt/mathematical_optimization/backend_selection.hpp
• cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem.hpp
• cpp/include/cuopt/mathematical_optimization/cpu_optimization_problem_solution.hpp
• cpp/include/cuopt/mathematical_optimization/cpu_pdlp_warm_start_data.hpp
• cpp/include/cuopt/mathematical_optimization/cuopt_c.h
• cpp/include/cuopt/mathematical_optimization/io/data_model_view.hpp
• cpp/include/cuopt/mathematical_optimization/io/mps_data_model.hpp
• cpp/include/cuopt/mathematical_optimization/io/mps_writer.hpp
• cpp/include/cuopt/mathematical_optimization/io/parser.hpp
• cpp/include/cuopt/mathematical_optimization/io/utilities/cython_parser.hpp
• cpp/include/cuopt/mathematical_optimization/io/writer.hpp
• cpp/include/cuopt/mathematical_optimization/mip/solver_settings.hpp
• cpp/include/cuopt/mathematical_optimization/mip/solver_solution.hpp
• cpp/include/cuopt/mathematical_optimization/optimization_problem.hpp
• cpp/include/cuopt/mathematical_optimization/optimization_problem_solution.hpp
• cpp/include/cuopt/mathematical_optimization/pdlp/pdlp_warm_start_data.hpp
• cpp/include/cuopt/mathematical_optimization/pdlp/solver_settings.hpp
• cpp/include/cuopt/mathematical_optimization/pdlp/solver_solution.hpp
• cpp/include/cuopt/mathematical_optimization/solve.hpp
• cpp/include/cuopt/mathematical_optimization/solve_remote.hpp
• cpp/include/cuopt/mathematical_optimization/solver_settings.hpp
• cpp/include/cuopt/mathematical_optimization/utilities/cython_solve.hpp
• cpp/include/cuopt/mathematical_optimization/utilities/cython_types.hpp
• cpp/include/cuopt/routing/assignment.hpp
• cpp/include/cuopt/routing/cython/cython.hpp
• cpp/include/cuopt/routing/data_model_view.hpp
• cpp/include/cuopt/routing/routing_structures.hpp
• cpp/include/cuopt/routing/solve.hpp
• cpp/include/cuopt/routing/solver_settings.hpp
• cpp/src/grpc/client/solve_remote.cpp
• cpp/src/grpc/grpc_problem_mapper.cpp
• cpp/src/grpc/grpc_settings_mapper.cpp
• cpp/src/grpc/grpc_solution_mapper.cpp
• cpp/src/io/data_model_view.cpp
• cpp/src/io/lp_parser.cpp
• cpp/src/io/mps_data_model.cpp
• cpp/src/io/mps_writer.cpp
• cpp/src/io/parser.cpp
• cpp/src/io/writer.cpp
• cpp/src/math_optimization/solution_reader.hpp
• cpp/src/math_optimization/solver_settings.cu
• cpp/src/mip_heuristics/solve.cu
• cpp/src/mip_heuristics/solver_settings.cu
• cpp/src/mip_heuristics/solver_solution.cu
• cpp/src/pdlp/cpu_optimization_problem.cpp
• cpp/src/pdlp/cpu_pdlp_warm_start_data.cu
• cpp/src/pdlp/optimization_problem.cu
• cpp/src/pdlp/pdlp.cu
• cpp/src/pdlp/pdlp_warm_start_data.cu
• cpp/src/pdlp/solution_conversion.cu
• cpp/src/pdlp/solve.cu
• cpp/src/pdlp/solver_settings.cu
• cpp/src/pdlp/solver_solution.cu
• cpp/src/routing/assignment.cu
• cpp/src/routing/data_model_view.cu
• cpp/src/routing/distance_engine/waypoint_matrix.cpp
• cpp/src/routing/generator/generator.hpp
• cpp/src/routing/solve.cu
• cpp/src/routing/solver_settings.cu
• cpp/src/routing/utilities/cython.cu
• cpp/src/utilities/logger.hpp
• cpp/tests/CMakeLists.txt
• cpp/tests/examples/routing/CMakeLists.txt
• cpp/tests/linear_programming/CMakeLists.txt
• cpp/tests/linear_programming/grpc/CMakeLists.txt
• cpp/tests/routing/CMakeLists.txt

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n== ci/check_symbols.sh (relevant lines) ==\n'
cat -n ci/check_symbols.sh | sed -n '1,180p'

printf '\n== occurrences of pslp_ and pattern handling ==\n'
rg -n 'pslp_|has_trailing_scope|index\\(symbol, pattern \\|\\| index\\(symbol, pattern "::"\\) == 1' -S .

Repository: NVIDIA/cuopt

Length of output: 3260

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== ci/check_symbols.sh excerpt =="
sed -n '50,80p' ci/check_symbols.sh | cat -n

echo
echo "== pslp_ references =="
rg -n '\bpslp_' ci . || true

Repository: NVIDIA/cuopt

Length of output: 4006

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '55,70p' ci/check_symbols.sh

Repository: NVIDIA/cuopt

Length of output: 621

---

Make pslp_ match the prefix. pslp_ goes through the non-:: branch, so it only matches the exact symbol pslp_ or symbols starting with pslp_::; plain prefixed symbols like pslp_foo are skipped. If this entry is meant to cover the PSLP symbol prefix, add explicit prefix handling and a regression fixture.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ci/check_symbols.sh` around lines 61 - 64, Update the non-trailing-scope
matching logic in has_trailing_scope so the pslp_ pattern matches symbols
beginning with the plain prefix pslp_, while preserving existing exact and
::-qualified matching behavior for other patterns. Add a regression fixture
covering a symbol such as pslp_foo.

Source: Path instructions

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Use the repository-required include-guard form.

Both changed headers use #pragma once, while the supplied coding guideline requires #define include guards.

• cpp/include/cuopt/export.hpp#L8-L8: replace #pragma once with a unique #ifndef/#define guard and closing #endif.
• cpp/include/cuopt/routing/assignment.hpp#L8-L8: replace #pragma once with a unique #ifndef/#define guard and closing #endif.

As per coding guidelines, C++ headers must use #define include guards.

📍 Affects 2 files

• cpp/include/cuopt/export.hpp#L8-L8 (this comment)
• cpp/include/cuopt/routing/assignment.hpp#L8-L8

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/include/cuopt/export.hpp` at line 8, Replace `#pragma` once with a unique
`#ifndef/`#define include guard and matching closing `#endif` in
cpp/include/cuopt/export.hpp (lines 8-8) and
cpp/include/cuopt/routing/assignment.hpp (lines 8-8), ensuring each header uses
a distinct guard name.

Source: Coding guidelines

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Link the gRPC tests against the shared cuopt target.

All three targets still use cuopt_static, which contradicts this PR’s shared-library test-linkage refactor and bypasses validation of the new exported symbols. Replace each occurrence with cuopt.

Proposed fix

-    cuopt_static
+    cuopt

Apply this change at all three locations.

Also applies to: 70-70, 109-109

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tests/linear_programming/grpc/CMakeLists.txt` at line 27, Replace all
three cuopt_static references in the gRPC test target definitions with cuopt,
ensuring every test links against the shared target and validates its exported
symbols.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

cpp/tests/CMakeLists.txt (1)

25-28: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Avoid hard-coding cuopt in cuopttestutils.
This static utility can pull cuopt onto the test link line, so STATIC_LIB tests may end up with both cuopt_static and cuopt. Link the utility against ${_cuopt_link_target} instead, or drop the dependency here so the selected target stays consistent.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cpp/tests/CMakeLists.txt` around lines 25 - 28, Update the test utility link
dependencies around the GTest entries to avoid hard-coding cuopt; use
${_cuopt_link_target} or remove the dependency so STATIC_LIB tests do not link
both cuopt_static and cuopt and the selected target remains consistent.

Source: Path instructions

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@cpp/tests/CMakeLists.txt`:
- Around line 25-28: Update the test utility link dependencies around the GTest
entries to avoid hard-coding cuopt; use ${_cuopt_link_target} or remove the
dependency so STATIC_LIB tests do not link both cuopt_static and cuopt and the
selected target remains consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2f330ce6-98bd-4f33-a910-1bfebc743b6d

📥 Commits

Reviewing files that changed from the base of the PR and between 8d7160b and 74dfed7.

📒 Files selected for processing (28)

• cpp/src/cuts/cuts.cpp
• cpp/src/dual_simplex/presolve.cpp
• cpp/src/dual_simplex/right_looking_lu.cpp
• cpp/src/dual_simplex/scaling.cpp
• cpp/src/dual_simplex/solve.cpp
• cpp/src/io/mps_parser.cpp
• cpp/src/linear_algebra/sparse_matrix.cpp
• cpp/src/linear_algebra/sparse_vector.cpp
• cpp/src/math_optimization/tic_toc.hpp
• cpp/src/mip_heuristics/feasibility_jump/feasibility_jump.cu
• cpp/src/mip_heuristics/presolve/bounds_presolve.cu
• cpp/src/mip_heuristics/presolve/load_balanced_bounds_presolve.cu
• cpp/src/mip_heuristics/presolve/multi_probe.cu
• cpp/src/mip_heuristics/presolve/third_party_presolve.cpp
• cpp/src/mip_heuristics/problem/load_balanced_problem.cu
• cpp/src/mip_heuristics/problem/problem.cu
• cpp/src/mip_heuristics/solution/solution.cu
• cpp/src/mip_heuristics/solver.cu
• cpp/src/pdlp/cusparse_view.cu
• cpp/src/routing/ges_solver.cu
• cpp/src/routing/local_search/local_search.cu
• cpp/src/routing/problem/problem.cu
• cpp/src/routing/solution/solution.cu
• cpp/src/routing/solver.cu
• cpp/tests/CMakeLists.txt
• cpp/tests/linear_programming/CMakeLists.txt
• cpp/tests/mip/CMakeLists.txt
• cpp/tests/routing/CMakeLists.txt

🚧 Files skipped from review as they are similar to previous changes (1)

• cpp/tests/routing/CMakeLists.txt

[ramakrishnap-nv]

Closing to rethink approach. The mix of CUOPT_INTERNAL_EXPORT + static-linked tests is not clean. Plan: assess which tests depend on internal APIs, determine if they're valuable, and consolidate into single per-module internal test binaries (one for routing, one for numerical optimization) that link against cuopt_static.