Reviewing files that changed from the base of the PR and between c1b2e74 and e6cf116.

• NEWS.md (1 hunks)
• doc/src/release_notes.rst (1 hunks)
• include/contraction/contract.hpp (1 hunks)
• include/contraction/contractionGraph.hpp (3 hunks)
• include/contraction/deadEndContraction.hpp (3 hunks)
• include/contraction/linearContraction.hpp (3 hunks)
• include/cpp_common/ch_edge.hpp (1 hunks)
• include/cpp_common/ch_vertex.hpp (1 hunks)
• locale/en/LC_MESSAGES/pgrouting_doc_strings.po (2 hunks)
• locale/pot/pgrouting_doc_strings.pot (2 hunks)
• src/common/ch_edge.cpp (2 hunks)
• src/common/ch_vertex.cpp (2 hunks)
• src/contraction/contractGraph_driver.cpp (1 hunks)

• GitHub Check: Codacy Static Code Analysis
• GitHub Check: Ubuntu Boost (83)
• GitHub Check: Ubuntu Boost (79)
• GitHub Check: Ubuntu Boost (80)
• GitHub Check: Ubuntu Boost (78)
• GitHub Check: Ubuntu Boost (86)
• GitHub Check: Ubuntu Boost (56)
• GitHub Check: Ubuntu Boost (84)
• GitHub Check: Ubuntu Boost (77)
• GitHub Check: Check queries
• GitHub Check: Ubuntu Boost (68)
• GitHub Check: Ubuntu Boost (75)
• GitHub Check: Ubuntu Boost (76)
• GitHub Check: Ubuntu clang
• GitHub Check: macos (macos-14)
• GitHub Check: macos (macos-latest)
• GitHub Check: documentation (en)

locale/en/LC_MESSAGES/pgrouting_doc_strings.po (2)

11-11: Verify the POT-Creation-Date.

The POT-Creation-Date is set to "2025-04-17", which appears to be a future date. Please verify if this is intentional or if it should be adjusted to the current date.

---

4011-4017: LGTM! Documentation updates for code enhancement.

The new message entries properly document the code enhancements related to issue #2802 for the pgr_contraction family of functions. This aligns well with the PR objectives of code reorganization without altering functionality.

doc/src/release_notes.rst (1)

90-94: Add “C/C++ code enhancement” section for issue #2802
The new rubric and entry correctly document the internal code refactor for the pgr_contraction family under pgRouting 3.8.0. The link and description align with the existing format.

NEWS.md (1)

60-64: Document internal code factorization in NEWS.md
The bold heading and bullet item for #2802 appropriately mirror the release notes, ensuring that both RST and Markdown changelogs stay in sync.

🧰 Tools

🪛 markdownlint-cli2 (0.17.2)

60-60: Emphasis used instead of a heading
null

(MD036, no-emphasis-as-heading)

locale/pot/pgrouting_doc_strings.pot (2)

11-11: Updated POT-Creation-Date to reflect recent changes.

The POT creation date has been updated to April 17, 2025, indicating this is a recent update to the translation template file.

---

3586-3591: New documentation entries for code enhancements.

These new entries document the code factorization work done on the pgr_contraction family of functions (issue #2802). This properly reflects the internal code improvements in the documentation and translation strings.

The addition of these entries is important for maintaining documentation completeness, as it informs users and translators about the code improvements that have been made, even though they don't change the public API.

include/cpp_common/ch_edge.hpp (2)

50-51: Method signature looks good, interface extension is consistent.

The addition of the set_contracted_vertices method provides a clean way to replace the entire collection of contracted vertices at once. This complements the existing API that allows for individual vertex additions.

---

56-56: Method provides batch addition capability, good enhancement.

This method allows adding multiple contracted vertices at once, which is more efficient than adding them one by one. The signature is consistent with the similar operation in the CH_vertex class.

include/contraction/contract.hpp (1)

109-109: Improved architecture - centralized forbidden vertex management.

This change properly delegates the responsibility of managing forbidden vertices to the graph object instead of the contractor. This centralization aligns with good design principles by keeping related functionality together and reduces duplication across different contractor implementations.

src/common/ch_edge.cpp (2)

34-37: Implementation for set_contracted_vertices looks correct.

The implementation directly assigns the provided identifiers collection to the member variable, which is the expected behavior for a setter method. This matches the implementation pattern used in the CH_vertex class.

---

77-79: Implementation for add_contracted_vertices looks correct.

The implementation uses the += operator to append the provided identifiers to the existing collection, which is consistent with other vertex addition methods in this class. The pattern is clean and efficient.

src/contraction/contractGraph_driver.cpp (1)

81-82: Code simplification through proper encapsulation.

Replacing direct access to graph internals with calls to dedicated methods improves encapsulation and maintainability. This change removes implementation details from this file and delegates responsibility to the graph class, which is the proper owner of this functionality.

src/common/ch_vertex.cpp (1)

37-40: Default‑ctor looks good but add a short‑circuit return
The explicit initialisation of vertex_order and metric to -1 removes UB that would appear later when these fields are read. Nice!

include/contraction/deadEndContraction.hpp (1)

79-81:

❓ Verification inconclusive

Check the result of get_min_cost_edge
get_min_cost_edge returns (edge, found) but the found flag is ignored.
If no edge exists between u and v, found is false and edge contains default values; you will still merge its (empty) contracted‑vertex list into graph[u].
While harmless today, this silently masks topology errors and could break when additional properties are copied later.

-const auto& e = graph.get_min_cost_edge(u, v);
-graph[u].contracted_vertices() += std::get<0>(e).contracted_vertices();
-graph[u].add_contracted_vertex(graph[v]);
+auto [minEdge, found] = graph.get_min_cost_edge(u, v);
+if (found) {
+    graph[u].contracted_vertices() += minEdge.contracted_vertices();
+    graph[u].add_contracted_vertex(graph[v]);
+}

Please confirm that silently skipping missing edges is acceptable; otherwise the loop should throw or log.

---

Check the found flag from get_min_cost_edge
get_min_cost_edge(u, v) returns (edge, found). If found is false, you’ll still merge a default‐constructed edge’s contracted‑vertex list, silently masking missing links. Either skip the merge, log a warning, or throw an exception.

• File: include/contraction/deadEndContraction.hpp (lines 79–81)

-const auto& e = graph.get_min_cost_edge(u, v);
-graph[u].contracted_vertices() += std::get<0>(e).contracted_vertices();
-graph[u].add_contracted_vertex(graph[v]);
+auto [minEdge, found] = graph.get_min_cost_edge(u, v);
+if (found) {
+    graph[u].contracted_vertices() += minEdge.contracted_vertices();
+    graph[u].add_contracted_vertex(graph[v]);
+} else {
+    // TODO: log a warning or throw to avoid masking missing edges
+}

include/cpp_common/ch_vertex.hpp (1)

51-52: Keep signature in sync with implementation
After changing the .cpp version, remember to update the declaration:

-void set_contracted_vertices(Identifiers<int64_t>&);
+void set_contracted_vertices(const Identifiers<int64_t>&);

include/contraction/contractionGraph.hpp (1)

90-132: get_min_cost_edge aggregates contracted vertices from all parallel edges

contracted_vertices is filled inside the loop before the minimum‑cost edge is known.
If multiple parallel edges (u→v) exist, vertices contracted on non‑minimal edges leak into the result.

-    contracted_vertices += this->graph[e].contracted_vertices();
-    if (this->graph[e].cost < min_cost) {
-        min_cost = this->graph[e].cost;
-        edge = this->graph[e];
-        found = true;
-    }
+    const auto &cv = this->graph[e].contracted_vertices();
+    if (this->graph[e].cost < min_cost) {
+        min_cost         = this->graph[e].cost;
+        edge             = this->graph[e];
+        contracted_vertices = cv;         // keep only the winning edge’s vertices
+        found            = true;
+    } else {
+        contracted_vertices += cv;        // optional: keep union of all, if intended
+    }

Please double‑check the intended behaviour and adjust accordingly.