add QCQP support for grpc#1320
Open
tmckayus wants to merge 9 commits into
Open
Conversation
Extend the ArrayChunk proto message so a single chunk can target either:
- a top-level array (existing behavior), or
- an array inside a repeated nested message, identified by
(container_field_num, container_index, field_id).
…iner chunks Adds quadratic_constraints (QCQP rows) to the codegen registry as a repeated_messages entry on OptimizationProblem, regenerates the conversion code, and wires the C++ transport for container-keyed chunks so QC arrays of unbounded size travel the same chunked path as top-level arrays.
Covers the QCQP transport at three layers: * Mapper unit tests (grpc_client_test.cpp) * Pipe-serialization unit test (grpc_pipe_serialization_test.cpp): * Integration tests (grpc_integration_test.cpp)
tmckayus
added
feature request
Contributor
Author
|
Note, this PR should merge after #1290 so that we can test a true end-to-end solve with grpc |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds repeated-message QCQP transport and container-aware chunked-array support: codegen emits nested QC proto/conversions; client stamps container metadata on array chunks; pipe serialization carries container sections; server maps container arrays into problems; tests cover unary, chunked, and validation cases. ChangesQuadratic Constraints gRPC Transport
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
cpp/src/grpc/codegen/generate_conversions.py (1)
2810-2832:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExtend
--auto-numberand field-number detection to coverrepeated_messages.These helpers still only walk
scalars/arrays/fields/warm_start. After--strip, the new QCQP registry leavesrepeated_messages.quadratic_constraintsand its inner fields unnumbered, so_iter_repeated_messages()will fail on the missing parentfield_numand the strip → auto-number workflow no longer works for this shape.Also applies to: 2835-3015
🤖 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/src/grpc/codegen/generate_conversions.py` around lines 2810 - 2832, _registry_has_field_numbers currently only inspects "scalars", "arrays", "fields", and "warm_start" and misses the new repeated_messages structure; update this function (and the related helpers around _iter_repeated_messages in the 2835-3015 range) to also walk any "repeated_messages" entries and their nested lists (e.g., "quadratic_constraints" and their inner field lists), invoking _collect_field_nums (for "field_num" and "array_id") and _collect_settings_field_nums where appropriate so that presence of field numbers in repeated_messages is detected; similarly adjust _iter_repeated_messages to expect/handle parent field_num/array_id on repeated_messages so auto-number/strip→auto-number flows work for the new shape.cpp/src/grpc/server/grpc_service_impl.cpp (1)
164-179:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject chunks whose
total_elementsdisagrees with earlier chunks.Once
metais initialized, later chunks for the same top-level/container array can send a differenttotal_elementsand still pass the bounds checks here. That leaves malformed uploads to fail only after the job is enqueued andwrite_chunked_request_to_pipe()tries to reassemble them. Compare each subsequent chunk againstmeta.total_elementsand returnINVALID_ARGUMENTimmediately. As per coding guidelines, gRPC server C++ code requires input validation on all request fields reaching the solver.🤖 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/src/grpc/server/grpc_service_impl.cpp` around lines 164 - 179, The code currently only sets meta.total_elements when it is zero but does not reject later chunks that provide a conflicting total_elems; add an explicit validation: when meta.total_elements != 0 and total_elems != meta.total_elements, return Status(StatusCode::INVALID_ARGUMENT, "...") to reject the chunk immediately. Update the logic around meta.total_elements / total_elems in the same block (near array_field_element_size, field_id, kMaxChunkedArrayBytes checks) so any subsequent chunk with mismatched total_elements is validated before enqueueing or calling write_chunked_request_to_pipe().
🤖 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 `@cpp/src/grpc/codegen/generate_conversions.py`:
- Around line 1703-1745: The repeated-message decode loop for entries (the block
iterating _iter_repeated_messages and building _entry before
cpu_problem.{setter}) lacks validation that companion arrays (e.g.,
linear_*_values vs linear_*_indices, quadratic_*_values vs quadratic_*_indices
or any paired arrays discovered by _repeated_message_arrays) have matching
lengths, so malformed requests can create inconsistent rows; update the loop
that fills _entry (in the repeated_messages handling using pb_problem.{name}(),
cpp_inner, and setter) to validate each companion pair's sizes on pb_entry (and
any scalar/array length relationships implied by body metadata) and if a
mismatch is detected either skip pushing that _entry and continue or return/fail
the request consistently (align with existing top-level CSR behavior), ensuring
you perform these checks before _entries.push_back(std::move(_entry)) and before
calling cpu_problem.{setter}(std::move(_entries)).
In `@cpp/src/grpc/cuopt_remote_service.proto`:
- Around line 152-155: The comment describing the top-level array mode for the
message that mentions container_field_num, container_index, field_id and
ArrayFieldId still says "This is the only mode used today" but that is stale due
to the newly added container-targeted chunk mode; update the comment by removing
or rephrasing that sentence so it no longer claims exclusivity (e.g., delete
"This is the only mode used today." or replace it with a note stating other
modes such as container-targeted chunk mode exist) so the documentation for
container_field_num/container_index/field_id remains accurate.
- Around line 171-175: Add input validation that enforces both-or-neither
presence for the proto optional fields container_field_num and container_index:
in the RPC/request validation code that processes the message containing these
fields (e.g., the handler/validator for the Chunk-like message from
cuopt_remote_service.proto), check has_container_field_num() and
has_container_index() and reject requests where exactly one is set (returning an
INVALID_ARGUMENT/error with a clear message). Implement this check in the
request validation path before any routing/solver logic so partial presence
cannot be interpreted as index 0.
In `@cpp/src/grpc/grpc_chunk_key.hpp`:
- Around line 20-38: The struct ContainerArrayKey must be renamed to follow the
project's snake_case_t convention (e.g., container_array_key_t); update the type
name declaration and all internal member functions/operators (operator< and
operator==) to use the new name, and replace every use of ContainerArrayKey
across the codebase (constructors, typedefs, templates, function signatures,
containers, comparisons, etc.) to the new name so compilation and linkage remain
correct; ensure any forward-declarations and header includes referencing
ContainerArrayKey are also updated.
In `@cpp/src/grpc/server/grpc_pipe_serialization.hpp`:
- Around line 235-237: The check that compares num_container_arrays to
kMaxPipeArrayFields is too strict for QCQP because num_container_arrays counts
every nested repeated array; replace it with a dedicated limit and/or derive the
limit from the payload/header metadata instead of reusing kMaxPipeArrayFields.
Specifically, in the block that calls read_from_pipe(fd, &num_container_arrays,
...) and currently does "if (num_container_arrays > kMaxPipeArrayFields) return
false;", introduce a new constant (e.g., kMaxContainerArrays) or compute a max
from the message size/header repeated-message metadata (or payload byte limits)
and validate num_container_arrays against that value; update any comments and
tests that reference num_container_arrays validation accordingly.
In `@cpp/tests/linear_programming/grpc/grpc_client_test.cpp`:
- Around line 2319-2342: The helper assemble_chunk_requests currently memcpy's
ac.data() into dest using byte_offset and data_size without validating bounds;
before the std::memcpy call (in assemble_chunk_requests, using variables
byte_offset, ac.data().size(), and dest->size()) add an assertion/check that
byte_offset + ac.data().size() <= dest->size() (e.g. ASSERT_LE or equivalent
test-failure macro) and handle the failure path so the test fails cleanly rather
than invoking undefined behavior.
In `@cpp/tests/linear_programming/grpc/grpc_integration_test.cpp`:
- Around line 1239-1243: The test only asserts result.success after calling
client->solve_lp(problem, settings) but should also check that the returned
solution object is non-null; update the assertion block that references
client->solve_lp, result.success, and result.error_message to also assert
result.solution != nullptr (e.g. add an EXPECT_NE(result.solution, nullptr) or
EXPECT_TRUE(result.solution) immediately after the existing
EXPECT_TRUE(result.success) so the transport test verifies a valid solution
structure was returned).
- Around line 1276-1277: The test currently asserts result.success but doesn't
verify the returned solution pointer; update the grpc_integration_test to add a
null-check for the solution from client->solve_lp by asserting result.solution
!= nullptr (e.g., using EXPECT_NE or EXPECT_TRUE on result.solution) immediately
after EXPECT_TRUE(result.success) to match the established pattern used around
lines 1239-1243 and ensure transport validation is robust.
---
Outside diff comments:
In `@cpp/src/grpc/codegen/generate_conversions.py`:
- Around line 2810-2832: _registry_has_field_numbers currently only inspects
"scalars", "arrays", "fields", and "warm_start" and misses the new
repeated_messages structure; update this function (and the related helpers
around _iter_repeated_messages in the 2835-3015 range) to also walk any
"repeated_messages" entries and their nested lists (e.g.,
"quadratic_constraints" and their inner field lists), invoking
_collect_field_nums (for "field_num" and "array_id") and
_collect_settings_field_nums where appropriate so that presence of field numbers
in repeated_messages is detected; similarly adjust _iter_repeated_messages to
expect/handle parent field_num/array_id on repeated_messages so
auto-number/strip→auto-number flows work for the new shape.
In `@cpp/src/grpc/server/grpc_service_impl.cpp`:
- Around line 164-179: The code currently only sets meta.total_elements when it
is zero but does not reject later chunks that provide a conflicting total_elems;
add an explicit validation: when meta.total_elements != 0 and total_elems !=
meta.total_elements, return Status(StatusCode::INVALID_ARGUMENT, "...") to
reject the chunk immediately. Update the logic around meta.total_elements /
total_elems in the same block (near array_field_element_size, field_id,
kMaxChunkedArrayBytes checks) so any subsequent chunk with mismatched
total_elements is validated before enqueueing or calling
write_chunked_request_to_pipe().
🪄 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: b06bead2-09bd-4f9c-847b-457eaa8c6cea
⛔ Files ignored due to path filters (9)
cpp/src/grpc/codegen/generated/cuopt_remote_data.protois excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_array_field_element_size.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_build_array_chunks.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_chunked_arrays_to_problem.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_estimate_problem_size.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_populate_chunked_header_lp.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_populate_chunked_header_mip.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_problem_to_proto.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_proto_to_problem.incis excluded by!**/generated/**
📒 Files selected for processing (14)
cpp/src/grpc/codegen/field_registry.yamlcpp/src/grpc/codegen/generate_conversions.pycpp/src/grpc/cuopt_remote_service.protocpp/src/grpc/grpc_chunk_key.hppcpp/src/grpc/grpc_problem_mapper.cppcpp/src/grpc/grpc_problem_mapper.hppcpp/src/grpc/server/grpc_field_element_size.hppcpp/src/grpc/server/grpc_pipe_serialization.hppcpp/src/grpc/server/grpc_server_types.hppcpp/src/grpc/server/grpc_service_impl.cppcpp/src/grpc/server/grpc_worker.cppcpp/tests/linear_programming/grpc/grpc_client_test.cppcpp/tests/linear_programming/grpc/grpc_integration_test.cppcpp/tests/linear_programming/grpc/grpc_pipe_serialization_test.cpp
Contributor
|
Better to change "QPQC" to "QCQP" in the title of PR. |
Contributor
Author
thank you! |
* remove unused solution size estimators * add problem size estimator comments for better clarity * fix gaps in proto auto-numbering related to new QP support
|
Actionable comments posted: 0 |
add extra validation before writing to the worker so that malformed data does not cause the worker to hang or crash
There was a problem hiding this comment.
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 `@cpp/src/grpc/server/grpc_pipe_serialization.hpp`:
- Around line 247-257: The writer currently computes this_bytes
(ac.total_elements() * elem_size) and the container_fields list without
enforcing the reader's caps; before emitting any header bytes in functions
handling array/chunk headers (look for ac.total_elements(), elem_size,
this_bytes and container_fields.size()), validate that this_bytes <=
kMaxPipeArrayBytes and that container_fields.size() <= kMaxPipeContainerArrays
and return PipeWriteStatus::ValidationFailed if either check fails; also keep
the existing overflow/elem_size guards but place the kMax checks prior to
writing header/state changes so the server never serializes a request the reader
would reject.
- Around line 247-257: The current validation only runs when ac.total_elements()
> 0, letting malformed chunks with total_elements <= 0 slip through; update the
validation in the grpc_pipe_serialization.hpp logic to explicitly reject
negative total_elements and to validate zero-length chunks: if
ac.total_elements() < 0 return PipeWriteStatus::ValidationFailed; if
ac.total_elements() == 0 then require the payload data be empty (otherwise
return PipeWriteStatus::ValidationFailed); additionally ensure elem_size is
validated (elem_size <= 0 -> return PipeWriteStatus::ValidationFailed) for any
chunk before computing byte counts and keep the existing overflow guard using
elem_size and ac.total_elements().
In `@cpp/tests/linear_programming/grpc/grpc_pipe_serialization_test.cpp`:
- Around line 134-147: The helper pipe_is_empty should retry poll() when it is
interrupted by a signal (errno == EINTR) instead of treating any error as "not
empty"; update the implementation of pipe_is_empty to loop calling ::poll(&pfd,
1, 0) and if it returns -1 and errno == EINTR continue the loop, otherwise break
and then decide emptiness using rc == 0. Reference the existing function
pipe_is_empty and the use of ::poll and errno to locate where to add the retry
loop and maintain existing semantics for non-EINTR errors and successful
returns.
🪄 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: c78b7c0c-9ffa-409e-a045-dd00029ec902
📒 Files selected for processing (3)
cpp/src/grpc/server/grpc_pipe_serialization.hppcpp/src/grpc/server/grpc_server_threads.cppcpp/tests/linear_programming/grpc/grpc_pipe_serialization_test.cpp
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
cpp/src/grpc/server/grpc_pipe_serialization.hpp (1)
231-245:⚠️ Potential issue | 🟠 Major | ⚡ Quick winReject oversized field sets during insertion, not after binning.
The new cap check runs only after every chunk has already been inserted into
top_fields/container_fields. A malicious request with a huge number of distinct container keys will still make the server allocate those map nodes before returningValidationFailed, which defeats this guard as a resource-exhaustion limit.💡 Suggested fix
for (const auto& ac : chunks) { int64_t elem_size = 0; detail::FieldChunks* fi = nullptr; if (ac.has_container_field_num()) { cuopt::linear_programming::container_array_key_t key{ ac.container_field_num(), ac.container_index(), ac.field_id()}; - fi = &container_fields[key]; + auto [it, inserted] = container_fields.try_emplace(key); + if (inserted && container_fields.size() > kMaxPipeContainerArrays) { + return PipeWriteStatus::ValidationFailed; + } + fi = &it->second; elem_size = array_field_element_size(key.container_field_num, key.field_id); } else { - fi = &top_fields[ac.field_id()]; + auto [it, inserted] = top_fields.try_emplace(ac.field_id()); + if (inserted && top_fields.size() > kMaxPipeArrayFields) { + return PipeWriteStatus::ValidationFailed; + } + fi = &it->second; elem_size = array_field_element_size(-1, ac.field_id()); } fi->chunks.push_back(&ac); @@ - if (top_fields.size() > kMaxPipeArrayFields) return PipeWriteStatus::ValidationFailed; - if (container_fields.size() > kMaxPipeContainerArrays) { - return PipeWriteStatus::ValidationFailed; - }As per coding guidelines,
cpp/src/grpc/**: "Size limits on problem data to prevent resource exhaustion."Also applies to: 287-292
🤖 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/src/grpc/server/grpc_pipe_serialization.hpp` around lines 231 - 245, The current logic inserts every chunk into top_fields/container_fields before enforcing the cap, allowing resource exhaustion; change to detect whether a map entry would be new before inserting: for each ac in chunks, compute the container key (cuopt::linear_programming::container_array_key_t) or top field id, use map::find or count to check if the key already exists, and if it does not, verify that adding one more distinct field would not exceed the configured cap (the same limit enforced later where ValidationFailed is returned); only after that check create/insert the FieldChunks entry and then push_back the chunk, and apply the same pre-insert check for both top_fields and container_fields so no new map nodes are allocated for oversized requests.
🤖 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/src/grpc/server/grpc_pipe_serialization.hpp`:
- Around line 231-245: The current logic inserts every chunk into
top_fields/container_fields before enforcing the cap, allowing resource
exhaustion; change to detect whether a map entry would be new before inserting:
for each ac in chunks, compute the container key
(cuopt::linear_programming::container_array_key_t) or top field id, use
map::find or count to check if the key already exists, and if it does not,
verify that adding one more distinct field would not exceed the configured cap
(the same limit enforced later where ValidationFailed is returned); only after
that check create/insert the FieldChunks entry and then push_back the chunk, and
apply the same pre-insert check for both top_fields and container_fields so no
new map nodes are allocated for oversized requests.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 339b3f78-5e15-47b6-96d7-484d4f4dd185
📒 Files selected for processing (2)
cpp/src/grpc/server/grpc_pipe_serialization.hppcpp/tests/linear_programming/grpc/grpc_pipe_serialization_test.cpp
🚧 Files skipped from review as they are similar to previous changes (1)
- cpp/tests/linear_programming/grpc/grpc_pipe_serialization_test.cpp
tmckayus
commented
May 29, 2026
| @@ -113,22 +113,6 @@ cpu_mip_solution_t<i_t, f_t> map_proto_to_mip_solution( | |||
| #include "generated_proto_to_mip_solution.inc" | |||
| } | |||
Contributor
Author
There was a problem hiding this comment.
Note, these were removed because they are dead code from an earlier version. The server does not need a size estimator for solutions, it now knows the size already.
tmckayus
commented
May 29, 2026
| EXPECT_EQ(success_count.load(), num_clients); | ||
| } | ||
|
|
||
| // ============================================================================= |
Contributor
Author
There was a problem hiding this comment.
Note this comment/test will need to change after SOCP is merged so we can check the actual objective. Not crucial because we also have integration tests
rgsl888prabhu
approved these changes
May 29, 2026
mlubin
approved these changes
May 29, 2026
Contributor
mlubin
left a comment
mlubin
left a comment
There was a problem hiding this comment.
Are we clearly documenting the extent to which the chunking protocol makes our grpc server special? I wouldn't want a user to try to roll their own client.
| - constraint_row_name: | ||
| field_num: 2 | ||
| type: string | ||
| - constraint_row_type: |
Contributor
There was a problem hiding this comment.
"sense" or "constraint sense" is a more standard and specific name for this field
| # Quadratic constraints (QCQP). Each entry is a row with its own RHS plus | ||
| # the row's linear coefficients (linear_values/linear_indices) and quadratic | ||
| # term (quadratic_values + quadratic_indices/offsets CSR per row-of-rows). | ||
| # Per-entry arrays are unbounded in size so they travel via container chunks |
Contributor
There was a problem hiding this comment.
It's worth a comment why this is different from linear constraints, i.e., we pass a matrix that encodes all linear constraints so we don't need to chunk within a repeated field there.
| # Quadratic constraints (QCQP). Each entry is a row with its own RHS plus | ||
| # the row's linear coefficients (linear_values/linear_indices) and quadratic | ||
| # term (quadratic_values + quadratic_indices/offsets CSR per row-of-rows). | ||
| # Per-entry arrays are unbounded in size so they travel via container chunks |
Contributor
There was a problem hiding this comment.
I would have guessed that the chunking inside the quadratic constraints is premature given the scale of typical problems, but ok to keep this functionality now that we have it.
Contributor
Author
There was a problem hiding this comment.
@mlubin that could be. It was a pretty natural extension from the existing array chunking.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@cpp/src/grpc/codegen/FIELD_REGISTRY_REFERENCE.md`:
- Line 38: The TOC link for the heading "5.8 Map a sentinel value (`max()` ↔
`-1`)" uses punctuation characters that break the fragment; update either the
heading or the TOC entry so the fragment is punctuation-stable (e.g., change the
heading to "5.8 Map a sentinel value: max() ↔ -1" or "5.8 Map a sentinel value
(max to -1)" and then update the TOC entry to match that sanitized slug,
ensuring the fragment in the link (the text inside #...) exactly matches the
normalized heading text.
🪄 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: 1674a354-49b6-4238-99ca-a4e1250b4afc
⛔ Files ignored due to path filters (3)
cpp/src/grpc/codegen/generated/generated_populate_chunked_header_lp.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_populate_chunked_header_mip.incis excluded by!**/generated/**cpp/src/grpc/codegen/generated/generated_problem_to_proto.incis excluded by!**/generated/**
📒 Files selected for processing (5)
cpp/src/grpc/GRPC_CODE_GENERATION.mdcpp/src/grpc/GRPC_INTERFACE.mdcpp/src/grpc/codegen/FIELD_REGISTRY_REFERENCE.mdcpp/src/grpc/codegen/field_registry.yamlcpp/src/grpc/codegen/generate_conversions.py
✅ Files skipped from review due to trivial changes (1)
- cpp/src/grpc/GRPC_INTERFACE.md
| - [5.5 Add a `repeated_messages` entry](#55-add-a-repeated_messages-entry-nested-message-type) | ||
| - [5.6 Add a warm-start field (LP only)](#56-add-a-warm-start-field-lp-only) | ||
| - [5.7 Make a setting `optional`](#57-make-a-setting-optional) | ||
| - [5.8 Map a sentinel value (`max()` ↔ `-1`)](#58-map-a-sentinel-value-max--1) |
There was a problem hiding this comment.
Fix the broken TOC fragment for section 5.8.
The anchor at Line 38 doesn’t resolve cleanly (MD051), so the TOC link can break navigation. Use a punctuation-stable heading/fragment pair.
🔧 Suggested doc fix
-- [5.8 Map a sentinel value (`max()` ↔ `-1`)](`#58-map-a-sentinel-value-max--1`)
+- [5.8 Map a sentinel value (max to -1)](`#58-map-a-sentinel-value-max-to--1`)
...
-### 5.8 Map a sentinel value (`max()` ↔ `-1`)
+### 5.8 Map a sentinel value (max to -1)Also applies to: 858-859
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 38-38: Link fragments should be valid
(MD051, link-fragments)
🤖 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/src/grpc/codegen/FIELD_REGISTRY_REFERENCE.md` at line 38, The TOC link
for the heading "5.8 Map a sentinel value (`max()` ↔ `-1`)" uses punctuation
characters that break the fragment; update either the heading or the TOC entry
so the fragment is punctuation-stable (e.g., change the heading to "5.8 Map a
sentinel value: max() ↔ -1" or "5.8 Map a sentinel value (max to -1)" and then
update the TOC entry to match that sanitized slug, ensuring the fragment in the
link (the text inside #...) exactly matches the normalized heading text.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This change adds grpc transport for quadratic constraints, so that QCQP problems can be solved remotely.