Bug Fix: Fix graph construction in pgr_edgeColoring using base_graph

[ayushjariyal]

Fixes #3101.

This PR remove graph building logicand replaces it with base_graph::insert_edges() for consistent graph building.

Summary by CodeRabbit

• Refactor
  • Internal edge-coloring implementation simplified to use a consolidated undirected graph representation; public API and method signatures unchanged.
  • Reduced internal mapping complexity and improved error messaging for the coloring step, aiming for easier maintenance and more reliable behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: ca0c9dec-429c-461f-a55d-4b454a3e2c4d

📥 Commits

Reviewing files that changed from the base of the PR and between 1843ad7 and c87e32a.

📒 Files selected for processing (2)

• include/coloring/edgeColoring.hpp
• src/coloring/edgeColoring.cpp

---

Walkthrough

Pgr_edgeColoring is refactored to replace internal Boost adjacency_list and ID/descriptor mappings with a single pgrouting::UndirectedGraph graph. The header drops helper lookup functions and related includes; the implementation delegates edge insertion to graph.insert_edges() and uses an external color map for boost::edge_coloring.

Changes

Cohort / File(s)	Summary
Header Cleanup include/coloring/edgeColoring.hpp	Replaces Boost adjacency_list typedefs and removed descriptor/ID mapping helpers (get_boost_vertex, get_vertex_id, get_edge_id) and maps (id_to_V, V_to_id, E_to_id). Removes legacy includes and adds cpp_common/base_graph.hpp and <iosfwd>. Public API signatures unchanged.
Implementation Update src/coloring/edgeColoring.cpp	Constructor now calls graph.insert_edges(edges) instead of manual vertex/edge insertion and map management. boost::edge_coloring invoked with an external std::map + boost::make_assoc_property_map; results read from graph.graph[e_i].id and the external color map. Removed translation-unit helper implementations and adjusted error message context. Includes updated (cpp_common/identifiers.hpp removed, cpp_common/base_graph.hpp added, <map> added).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 I hopped through nodes and tucked maps away,
One tidy graph now leads the coloring play,
Edges find hues without extra tracks,
Simpler paths ahead—no looking back! 🎨🐇

🚥 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
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: replacing custom graph construction with base_graph in pgr_edgeColoring.
Linked Issues check	✅ Passed	The PR directly addresses issue #3101 by removing custom graph-building logic and adopting base_graph::insert_edges().
Out of Scope Changes check	✅ Passed	All changes are scoped to pgr_edgeColoring's graph construction, focusing on replacing custom logic with base_graph methods.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@include/coloring/edgeColoring.hpp`:
- Line 59: The namespace qualifier has an extra space: change the declaration
"pgrouting:: UndirectedGraph graph;" to use the correct qualifier without the
space so it reads "pgrouting::UndirectedGraph graph;"; update the token in
include/coloring/edgeColoring.hpp where the symbol pgrouting::UndirectedGraph is
declared to remove the stray space.

In `@include/cpp_common/basic_edge.hpp`:
- Line 48: Remove the dead member int64_t color from class Basic_edge: delete
the field declaration in Basic_edge and remove any remaining references to
Basic_edge::color (e.g., graph.graph[e_i].color) so the code uses the external
color_storage / color_map and edgeColoring logic exclusively; ensure
cp_members() and any serialization or copy logic no longer mention color and run
a build to verify no residual references remain.

In `@src/coloring/edgeColoring.cpp`:
- Around line 79-80: The file ends with the closing namespace braces for
functions and pgrouting but is missing a trailing newline; update the EOF of
src/coloring/edgeColoring.cpp so there is a single newline character after the
final "}" that closes the pgrouting::functions namespace to ensure
POSIX-compliant end-of-file.

🪄 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: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 454be25f-ff55-400f-8511-f10e4152d3db

📥 Commits

Reviewing files that changed from the base of the PR and between e06c177 and 89fcc12.

📒 Files selected for processing (3)

• include/coloring/edgeColoring.hpp
• include/cpp_common/basic_edge.hpp
• src/coloring/edgeColoring.cpp

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@include/coloring/edgeColoring.hpp`:
- Line 39: The header declares a friend operator using std::ostream (the friend
operator<< on EdgeColoring), but doesn't include <iosfwd> so the type is
undefined; add an `#include` <iosfwd> to include/coloring/edgeColoring.hpp (near
the top with other includes) so std::ostream is forward-declared and the friend
declaration compiles even when includes are ordered differently.

In `@src/coloring/edgeColoring.cpp`:
- Line 38: The translation unit uses std::map (used around the edge coloring
logic) but does not include <map>, relying on transitive includes; add an
explicit `#include` <map> near the other headers at the top of edgeColoring.cpp
(alongside existing includes such as "cpp_common/base_graph.hpp") so std::map is
properly declared for the code that references it.

🪄 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: Repository UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 768fd744-e04b-4e91-9765-081562980878

📥 Commits

Reviewing files that changed from the base of the PR and between 89fcc12 and 1843ad7.

📒 Files selected for processing (2)

• include/coloring/edgeColoring.hpp
• src/coloring/edgeColoring.cpp