Feat/tikz galleries - #98
Conversation
…ithms Se añade soporte para visualizar la estructura jerárquica de árboles R y R* (con sus MBR y cajas de entrada) en un plano TikZ, incluyendo consultas de intersección con rectángulos. Se implementan las funciones put_rtree_result, visualize_rtree y visualize_rtree_query, y se extiende la clase RTree (tpl_r_tree.H) con DebugNode, DebugSnapshot y el método debug_snapshot(). Se agregan pruebas unitarias y ejemplos completos de galería que incluyen quadtree y árboles R/R*.
…ithms Se añade soporte para visualizar la estructura jerárquica de árboles R y R* (con sus MBR y cajas de entrada) en un plano TikZ, incluyendo consultas de intersección con rectángulos. Se implementan las funciones put_rtree_result, visualize_rtree y visualize_rtree_query, y se extiende la clase RTree (tpl_r_tree.H) con DebugNode, DebugSnapshot y el método debug_snapshot(). Se agregan pruebas unitarias y ejemplos completos de galería que incluyen quadtree y árboles R/R*.
…ithms Se añade soporte para visualizar la estructura jerárquica de árboles R y R* (con sus MBR y cajas de entrada) en un plano TikZ, incluyendo consultas de intersección con rectángulos. Se implementan las funciones put_rtree_result, visualize_rtree y visualize_rtree_query, y se extiende la clase RTree (tpl_r_tree.H) con DebugNode, DebugSnapshot y el método debug_snapshot(). Se agregan pruebas unitarias y ejemplos completos de galería que incluyen quadtree y árboles R/R*.
…ithms Se añade soporte para visualizar la estructura jerárquica de árboles R y R* (con sus MBR y cajas de entrada) en un plano TikZ, incluyendo consultas de intersección con rectángulos. Se implementan las funciones put_rtree_result, visualize_rtree y visualize_rtree_query, y se extiende la clase RTree (tpl_r_tree.H) con DebugNode, DebugSnapshot y el método debug_snapshot(). Se agregan pruebas unitarias y ejemplos completos de galería que incluyen quadtree y árboles R/R*.
|
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:
WalkthroughSe añade una galería TikZ de 20 algoritmos geométricos, se amplía el ejemplo de estructuras espaciales con QuadTree, RTree y RStarTree, se incorpora ChangesVisualización espacial y soporte de depuración
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant RTree
participant DebugSnapshot
participant TikzPlane
RTree->>DebugSnapshot: Genera snapshot estructural
DebugSnapshot->>TikzPlane: Entrega nodos, cajas y aciertos
TikzPlane->>TikzPlane: Emite comandos TikZ
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
Adds visualization/debugging support for spatial trees (notably R-tree/R*-tree) and expands TikZ-based “gallery” examples to showcase computational-geometry algorithms and data structures.
Changes:
- Added
RTree::debug_snapshot()plus snapshot structs to expose full R-tree structure (payload-independent) for visualization/debugging. - Refactored TikZ geometry rendering to use Aleph
Array(and Aleph sorting utilities) instead ofstd::vectorin core TikZ code paths. - Extended
tikzgeom_algorithms.Hwith helpers to render additional algorithm outputs (including R-tree rendering), plus new tests and example galleries.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tpl_r_tree.H | Adds debug snapshot structs and debug_snapshot() API for R-tree visualization. |
| tikzgeom.H | Replaces std::vector usage with Array and switches ordering to Aleph sort utils. |
| tikzgeom_algorithms.H | Adds visualization helpers (incl. R-tree) and refactors formatting/structure. |
| Tests/tikzgeom_algorithms_test.cc | Adds visualization tests for R-tree and R*-tree query rendering. |
| Tests/r_tree_test.cc | Adds invariant tests validating RTree::DebugSnapshot structure/bboxes. |
| Tests/r_star_tree_test.cc | Adds invariant tests validating RStarTree::DebugSnapshot structure/bboxes. |
| Examples/tikz_tree_structures_example.cc | Extends tree-structure gallery (quadtree + R-tree/R*-tree visuals). |
| Examples/tikz_computational_geometry_gallery_example.cc | Adds a new multi-panel computational geometry TikZ gallery example. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Tests/r_tree_test.cc (1)
207-259: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtraer
union_of/check_snapshot_nodea un helper de test compartido. Ambos archivos definen exactamente la misma lógica de verificación de invariantes deDebugSnapshoten unnamespaceanónimo local, duplicando el código.
Tests/r_tree_test.cc#L207-L259: moverunion_ofycheck_snapshot_node<Snapshot>a un header de utilidades de test compartido (p. ej.Tests/r_tree_test_utils.H) e incluirlo aquí.Tests/r_star_tree_test.cc#L208-L259: eliminar la copia duplicada e incluir el mismo header de utilidades compartido.🤖 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 `@Tests/r_tree_test.cc` around lines 207 - 259, Extrae union_of y check_snapshot_node<Snapshot> a un header compartido de utilidades de tests, preservando exactamente la lógica de verificación de DebugSnapshot; en Tests/r_tree_test.cc, líneas 207-259, mueve ambas funciones al helper e incluye dicho header, y en Tests/r_star_tree_test.cc, líneas 208-259, elimina la copia local e incluye el mismo helper.tpl_r_tree.H (1)
1103-1147: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDocumentar la profundidad de pila para la recursión en
debug_snapshot().El comentario Doxygen documenta la complejidad temporal (
O(n)) pero no la profundidad de la pila de llamadas del DFS recursivo (self(self, ...)), que esO(height_). Esto es requerido explícitamente por las guías de codificación para algoritmos recursivos.📝 Sugerencia de documentación
/** `@brief` Capture the full tree structure for visualization/debugging. * `@return` A `@ref` DebugSnapshot with every node in preorder; empty * (`nodes` empty, `root` unset) when the tree `is_empty()`. * `@par` Complexity O(n). + * `@note` Recursive DFS; call-stack depth is O(height_), bounded by the + * tree's height. * `@throws` std::bad_alloc if node allocation fails. */Como indican las guías de codificación, "Document space complexity for recursive algorithms (call stack depth) in Doxygen".
🤖 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 `@tpl_r_tree.H` around lines 1103 - 1147, Update the Doxygen comment for debug_snapshot() to document the recursive DFS call-stack depth as O(height_), alongside the existing O(n) time complexity. Do not change the implementation or other complexity descriptions.Source: Coding guidelines
🤖 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 `@Examples/tikz_computational_geometry_gallery_example.cc`:
- Around line 434-436: Replace the panels container in the relevant example code
with Aleph’s Array<Tikz_Plane>, preserving the reserve capacity of 20. Update
all panels operations, including emplace_back, back, and operator[], to use the
corresponding Array APIs while leaving the existing geometry behavior unchanged.
- Around line 696-710: Update the output-generation flow around the stream used
by the panel draw loop to explicitly close it and validate its final state after
writing the document footer. Only print the “Generated” success message and
compile hint when closing and stream validation succeed; otherwise report the
write/close failure and avoid claiming successful generation.
---
Nitpick comments:
In `@Tests/r_tree_test.cc`:
- Around line 207-259: Extrae union_of y check_snapshot_node<Snapshot> a un
header compartido de utilidades de tests, preservando exactamente la lógica de
verificación de DebugSnapshot; en Tests/r_tree_test.cc, líneas 207-259, mueve
ambas funciones al helper e incluye dicho header, y en
Tests/r_star_tree_test.cc, líneas 208-259, elimina la copia local e incluye el
mismo helper.
In `@tpl_r_tree.H`:
- Around line 1103-1147: Update the Doxygen comment for debug_snapshot() to
document the recursive DFS call-stack depth as O(height_), alongside the
existing O(n) time complexity. Do not change the implementation or other
complexity descriptions.
🪄 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: Pro
Run ID: 9fd7b5cc-f468-4926-b694-21def1e843cd
📒 Files selected for processing (8)
Examples/tikz_computational_geometry_gallery_example.ccExamples/tikz_tree_structures_example.ccTests/r_star_tree_test.ccTests/r_tree_test.ccTests/tikzgeom_algorithms_test.cctikzgeom.Htikzgeom_algorithms.Htpl_r_tree.H
…los y tests
- En tikz_computational_geometry_gallery_example.cc: cerrar el archivo de salida y verificar errores de escritura para evitar fallos silenciosos.
- En tpl_r_tree.H: corregir error tipográfico en documentación ("movable-only usable" → "move-only") y mejorar la descripción de complejidad de debug_snapshot.
- En tikz_tree_structures_example.cc: cambiar iteración no-const a const para corrección constante.
- En Tests/r_tree_test.cc y Tests/r_star_tree_test.cc: extraer funciones auxiliares duplicadas a un nuevo archivo cabecera r_tree_debug_snapshot_test_helpers.H y unificar su uso, eliminando código repetido.
…los y tests
- En tikz_computational_geometry_gallery_example.cc: cerrar el archivo de salida y verificar errores de escritura para evitar fallos silenciosos.
- En tpl_r_tree.H: corregir error tipográfico en documentación ("movable-only usable" → "move-only") y mejorar la descripción de complejidad de debug_snapshot.
- En tikz_tree_structures_example.cc: cambiar iteración no-const a const para corrección constante.
- En Tests/r_tree_test.cc y Tests/r_star_tree_test.cc: extraer funciones auxiliares duplicadas a un nuevo archivo cabecera r_tree_debug_snapshot_test_helpers.H y unificar su uso, eliminando código repetido.
…los y tests
- En tikz_computational_geometry_gallery_example.cc: cerrar el archivo de salida y verificar errores de escritura para evitar fallos silenciosos.
- En tpl_r_tree.H: corregir error tipográfico en documentación ("movable-only usable" → "move-only") y mejorar la descripción de complejidad de debug_snapshot.
- En tikz_tree_structures_example.cc: cambiar iteración no-const a const para corrección constante.
- En Tests/r_tree_test.cc y Tests/r_star_tree_test.cc: extraer funciones auxiliares duplicadas a un nuevo archivo cabecera r_tree_debug_snapshot_test_helpers.H y unificar su uso, eliminando código repetido.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
Tests/r_star_tree_test.cc:1
- The file header comment was accidentally changed from a block comment opener (
/*) to a line comment (//*). This leaves the following license header lines as raw tokens and also makes the later*/a stray token, which will break compilation.
//*
…los y tests
- En tikz_computational_geometry_gallery_example.cc: cerrar el archivo de salida y verificar errores de escritura para evitar fallos silenciosos.
- En tpl_r_tree.H: corregir error tipográfico en documentación ("movable-only usable" → "move-only") y mejorar la descripción de complejidad de debug_snapshot.
- En tikz_tree_structures_example.cc: cambiar iteración no-const a const para corrección constante.
- En Tests/r_tree_test.cc y Tests/r_star_tree_test.cc: extraer funciones auxiliares duplicadas a un nuevo archivo cabecera r_tree_debug_snapshot_test_helpers.H y unificar su uso, eliminando código repetido.
✅ Action performedReview finished.
|
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 `@Tests/r_tree_debug_snapshot_test_helpers.H`:
- Around line 43-56: Añade documentación Doxygen completa a los helpers union_of
y check_snapshot_node, incluyendo `@brief`, `@param`, `@return`, precondiciones,
garantías de excepción, complejidad temporal y espacial, y garantías de
thread-safety; mantén intacto el comportamiento actual de ambas funciones.
- Around line 80-86: Actualiza check_snapshot_node para recibir y reutilizar un
conjunto compartido de índices visitados durante toda la recursión; marca cada
nodo antes de descender y, si un índice ya aparece, registra una expectativa
fallida y retorna de forma controlada. Mantén la validación de límites existente
y evita contabilizar o procesar nodos repetidos.
- Around line 56-59: Valida idx en check_snapshot_node antes de llamar a
snap.nodes(idx), incluyendo el caso de snapshot vacío y cualquier índice fuera
de rango. Registra una aserción de prueba y retorna inmediatamente con un valor
apropiado cuando el índice no sea válido, preservando el acceso y las
comprobaciones existentes para índices válidos.
🪄 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: Pro
Run ID: a7d4e39f-027d-49ea-b4ca-1ea06c3bbcd3
📒 Files selected for processing (1)
Tests/r_tree_debug_snapshot_test_helpers.H
…los y tests
- En tikz_computational_geometry_gallery_example.cc: cerrar el archivo de salida y verificar errores de escritura para evitar fallos silenciosos.
- En tpl_r_tree.H: corregir error tipográfico en documentación ("movable-only usable" → "move-only") y mejorar la descripción de complejidad de debug_snapshot.
- En tikz_tree_structures_example.cc: cambiar iteración no-const a const para corrección constante.
- En Tests/r_tree_test.cc y Tests/r_star_tree_test.cc: extraer funciones auxiliares duplicadas a un nuevo archivo cabecera r_tree_debug_snapshot_test_helpers.H y unificar su uso, eliminando código repetido.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
Examples/tikz_computational_geometry_gallery_example.cc:436
- This example uses
std::vector<Tikz_Plane>even though the codebase policy is to prefer Aleph containers when an equivalent exists. Here the number of panels is known a priori (20), soArray<Tikz_Plane>(withreserve(20)andappend(...)) would satisfy the policy and keep container usage consistent across the repository.
std::vector<Tikz_Plane> panels;
panels.reserve(20);
auto & p = panels; // shorthand
Include tpl_array.H in r_tree_debug_snapshot_test_helpers.H to ensure proper compilation for Array<bool>.
Include tpl_array.H in r_tree_debug_snapshot_test_helpers.H to ensure proper compilation for Array<bool>.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
Examples/tikz_computational_geometry_gallery_example.cc:440
- This new example uses
std::vector<Tikz_Plane>for the panel collection, but the project container policy requires using Aleph containers when an equivalent exists. HereArray<Tikz_Plane>can be used, replacingemplace_back()/back()withappend(Tikz_Plane(...))andget_last()(or indexing withoperator()).
std::vector<Tikz_Plane> panels;
panels.reserve(20);
auto & p = panels; // shorthand
// 1. Primitives & polygons showcase.
p.emplace_back(190, 120, 6, 6);
{
Include tpl_array.H in r_tree_debug_snapshot_test_helpers.H to ensure proper compilation for Array<bool>.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
tpl_r_tree.H:1138
- Same readability issue as the leaf case:
Array<size_t> child_indices(node.children.size())sets capacity, not logical size. Switching to a default-constructedArray+reserve()makes it clear this is a push/append build-up, which helps prevent future maintenance mistakes with Aleph::Array’s non-std::vectorsizing semantics.
Array<size_t> child_indices(node.children.size());
for (size_t i = 0; i < node.children.size(); ++i)
child_indices.append(self(self, *node.children(i).child, depth + 1));
snap.nodes(out_idx).children = std::move(child_indices);
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ah-ranges.H (1)
571-581: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winInconsistencia de
std::moveentre las dos implementaciones deranges_fold_left.Aquí el fallback hace
init = op(init, elem), mientras que la variante de la sección!ALEPH_HAS_RANGES(líneas 913-918) haceinit = op(std::move(init), elem). Para tipos costosos de copiar o con sobrecargas que distinguen lvalue/rvalue el resultado y el rendimiento difieren según la configuración del compilador.♻️ Unificar el fallback
// Fallback for C++20 - use explicit variable to avoid rvalue issues for (auto &&elem : r) - init = op(init, elem); + init = op(std::move(init), elem); return init;🤖 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 `@ah-ranges.H` around lines 571 - 581, Unifica el comportamiento de ambos fallbacks de ranges_fold_left usando std::move(init) al invocar BinaryOp. Actualiza el bucle de ranges_fold_left para pasar init como rvalue, manteniendo intactas la ruta std::ranges::fold_left y el resto de la lógica.
🧹 Nitpick comments (10)
Tests/r_tree_test.cc (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUsar
std::ranges::sortpara ordenarArrayen los tests.En C++20 esta guía prioriza algoritmos de rangos, y
Arrayexpone iteradores STL adecuados (SortArrayutilizastd::ranges::sort). Cambiar estos 7 llamados astd::sort(...)porstd::ranges::sortmantendrá el comportamiento y alineará el test con el estilo del proyecto.🤖 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 `@Tests/r_tree_test.cc` at line 69, Replace all seven std::sort calls with std::ranges::sort while preserving their existing ranges and behavior: Tests/r_tree_test.cc lines 69, 77, and 88, and Tests/r_star_tree_test.cc lines 66, 77, 88, and 96. Use the existing Array iterators and project C++20 ranges style.Source: Coding guidelines
tpl_dynArray.H (2)
59-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
#include <ah-errors.H>duplicado.♻️ Limpieza
`#include` <ah-dry.H> `#include` <ah-errors.H> -#include <ah-errors.H> `#include` <tpl_array.H>🤖 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 `@tpl_dynArray.H` around lines 59 - 60, Remove the duplicate `#include` <ah-errors.H> directive, keeping a single inclusion in the header.
1382-1385: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueComparación con signo/sin signo en
has_curr().
curr_idxeslongyarray_ptr->size()essize_t; la comparación promuevecurr_idxasize_t. La guardacurr_idx >= 0previa evita el fallo lógico, pero el compilador emitirá-Wsign-compare. Unstatic_cast<size_t>(curr_idx) < array_ptr->size()deja la intención explícita.🤖 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 `@tpl_dynArray.H` around lines 1382 - 1385, Update has_curr() to avoid the signed/unsigned comparison warning by preserving the curr_idx >= 0 guard and explicitly converting curr_idx to size_t for the comparison with array_ptr->size(). Keep the existing null-pointer and bounds behavior unchanged.array_it.H (2)
216-217: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConstructor de conversión implícita.
Array_Iterator(const Array_Container<T> &)no esexplicit, por lo que cualquierArray_Container<T>se convierte silenciosamente en iterador en llamadas sobrecargadas. Considerar marcarloexplicitsalvo que la conversión implícita sea intencional para la API funcional de Aleph.🤖 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 `@array_it.H` around lines 216 - 217, Mark the Array_Iterator constructor accepting const Array_Container<T>& as explicit to prevent unintended implicit conversions during overload resolution, unless the library intentionally requires this conversion for its functional API.
410-411: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
const size_t nelimina implícitamente la asignación de copia/movimiento.Al ser un miembro no estático
const,Array_Container<T>deja de ser asignable (operator=implícito suprimido), lo que impide reasignar la vista o almacenarla en contenedores que requieran asignación. Para una vista no propietaria conviene un miembro noconsty exponer la inmutabilidad solo mediante la interfaz pública (size()/capacity()ya sonconst).♻️ Refactor propuesto
T *base = nullptr; - const size_t n = 0; + size_t n = 0;🤖 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 `@array_it.H` around lines 410 - 411, El miembro no estático const size_t n suprime la asignación implícita de Array_Container<T>. Declara n como miembro no const, manteniendo la inmutabilidad mediante la interfaz pública existente como size() y capacity(), y conserva sin cambios la semántica de la vista no propietaria.tpl_array.H (1)
473-498: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winSobrecargas ambiguas de
traverse.Coexisten
traverse(Operation&),traverse(Operation&) const,traverse(Operation&&) constytraverse(Operation&&). Con un lvalue no-const,Operation&yOperation&&(por deducción de referencia reenviada) compiten y la resolución depende de detalles sutiles; además los overloads&&con argumento por defectoOperation()no son invocables sin argumentos explícitos de plantilla. Vale la pena consolidar en un únicotemplate <class Op> bool traverse(Op &&op) const.🤖 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 `@tpl_array.H` around lines 473 - 498, Consolida las cuatro sobrecargas de traverse en una única plantilla `traverse(Op&& op) const`, eliminando las variantes con `Operation&`, la versión no const y los argumentos predeterminados. Conserva el reenvío perfecto hacia `array.traverse` y la firma const del contenedor.tpl_avlRk.H (2)
939-950: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueTruncamiento potencial al calcular
DIFFdesde alturas.
static_cast<signed char>(h2) - static_cast<signed char>(h1)convierte cada altura asigned charantes de restar; con alturas > 127 el resultado es incorrecto. Aunque un AVL consize_tde nodos difícilmente supere ~90 de altura, es más robusto restar como enteros y castear el resultado.🛡️ Ajuste sugerido
- DIFF(pivot) = static_cast<signed char>(h2) - static_cast<signed char>(h1); + DIFF(pivot) = + static_cast<signed char>(static_cast<long>(h2) - static_cast<long>(h1));Aplica igualmente en las líneas 987, 1000, 1035 y 1047.
🤖 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 `@tpl_avlRk.H` around lines 939 - 950, Actualiza los cálculos de DIFF en join_with_pivot y en las ubicaciones equivalentes indicadas para restar h1 y h2 como enteros de mayor rango, y solo después convertir el resultado al tipo de DIFF. Evita convertir cada altura a signed char antes de la resta, preservando el valor correcto para alturas superiores a 127.
780-1214: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftElimina o conecta las rutinas AVLRk sin llamadores.
join_exclusive_rec,join_with_pivot,join_right,join_left,split_key_rec,split_key_dup_rec,split_pos_rec,extract_minyextract_maxno intervienen en la API pública actual: sus versiones públicas usan recorridos iterativos y reinserción con costeO(n)/O(m log(n+m)). Para evitar mantener dos algoritmos divergentes y no documentados, conecta estas rutinasO(log n)a la API pública o elimina las auxiliares sin uso.🤖 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 `@tpl_avlRk.H` around lines 780 - 1214, Elimina las rutinas auxiliares sin llamadores visibles —extract_min, extract_max, join_exclusive_rec, join_with_pivot, join_right, join_left, split_key_rec, split_key_dup_rec y split_pos_rec— o intégralas en las implementaciones públicas actuales de join/split. Si las conectas, sustituye los recorridos iterativos y la reinserción por estas rutas O(log n), manteniendo la semántica pública existente; no dejes dos algoritmos divergentes.tpl_binNodeUtils.H (2)
55-57: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
inline staticen plantillas de ámbito de espacio de nombres es contradictorio.
staticfuerza enlace interno (una copia por unidad de traducción) y anula el propósito deinline; en plantillas ni siquiera es necesario, porque ya admiten definiciones múltiples. Mismo patrón en las líneas 100-102, 144-146, 799-801, 946-947, 1038, 1322 y 1776.♻️ Ajuste sugerido
template <class Node> -inline static void inorder_rec_helper(Node *node, const int &level, int &position, - void (*visitFct)(Node *, int, int)) +inline void inorder_rec_helper(Node *node, const int &level, int &position, + void (*visitFct)(Node *, int, int))🤖 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 `@tpl_binNodeUtils.H` around lines 55 - 57, Elimina el calificador static de inorder_rec_helper y conserva inline para evitar enlace interno innecesario en la plantilla. Aplica el mismo cambio a todas las funciones de plantilla con el patrón inline static indicadas en las líneas 100-102, 144-146, 799-801, 946-947, 1038, 1322 y 1776.
2647-2659: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win
reset_last()es O(n) por el recuento de nodos.
size(root)recorre todo el árbol solo para fijarpos, de modo que reposicionar el iterador al último elemento cuesta O(n) en lugar de O(log n). Conviene documentar la complejidad en el Doxygen del método o calcular la posición de forma perezosa (solo cuando se consulteget_pos()).🤖 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 `@tpl_binNodeUtils.H` around lines 2647 - 2659, El método reset_last() recorre todo el árbol mediante size(root) para establecer pos. Elimina ese recuento eager, deja la posición marcada como pendiente al reposicionar en advance_to_max(root) y haz que get_pos() calcule el índice solo cuando sea necesario, manteniendo el resultado correcto para el último elemento.
🤖 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 `@tpl_array.H`:
- Around line 456-459: Corrige Array::rev() para que devuelva el Array temporal
por valor, evitando retornar una referencia colgante tras reverse(). Elimina el
overload duplicado indicado y conserva un único overload const con el tipo de
retorno por valor.
In `@tpl_avl.H`:
- Around line 60-62: Remove the global using namespace Aleph directive before
namespace Aleph in tpl_avl.H at lines 60-62 and tpl_avlRk.H at line 62, leaving
only the namespace Aleph declaration in both headers.
In `@tpl_dynArray.H`:
- Around line 1387-1390: Actualiza los métodos is_last(), reset_last() y end()
para comprobar si array_ptr es nulo antes de acceder a él. Define el
comportamiento seguro para iteradores singulares de forma coherente con
has_curr(), evitando cualquier desreferencia nula y preservando el
comportamiento actual cuando array_ptr es válido.
- Around line 1302-1309: Update DynArray::reverse() to return immediately when
current_dim is zero, before initializing j with current_dim - 1. Preserve the
existing two-pointer swap behavior for non-empty arrays.
---
Outside diff comments:
In `@ah-ranges.H`:
- Around line 571-581: Unifica el comportamiento de ambos fallbacks de
ranges_fold_left usando std::move(init) al invocar BinaryOp. Actualiza el bucle
de ranges_fold_left para pasar init como rvalue, manteniendo intactas la ruta
std::ranges::fold_left y el resto de la lógica.
---
Nitpick comments:
In `@array_it.H`:
- Around line 216-217: Mark the Array_Iterator constructor accepting const
Array_Container<T>& as explicit to prevent unintended implicit conversions
during overload resolution, unless the library intentionally requires this
conversion for its functional API.
- Around line 410-411: El miembro no estático const size_t n suprime la
asignación implícita de Array_Container<T>. Declara n como miembro no const,
manteniendo la inmutabilidad mediante la interfaz pública existente como size()
y capacity(), y conserva sin cambios la semántica de la vista no propietaria.
In `@Tests/r_tree_test.cc`:
- Line 69: Replace all seven std::sort calls with std::ranges::sort while
preserving their existing ranges and behavior: Tests/r_tree_test.cc lines 69,
77, and 88, and Tests/r_star_tree_test.cc lines 66, 77, 88, and 96. Use the
existing Array iterators and project C++20 ranges style.
In `@tpl_array.H`:
- Around line 473-498: Consolida las cuatro sobrecargas de traverse en una única
plantilla `traverse(Op&& op) const`, eliminando las variantes con `Operation&`,
la versión no const y los argumentos predeterminados. Conserva el reenvío
perfecto hacia `array.traverse` y la firma const del contenedor.
In `@tpl_avlRk.H`:
- Around line 939-950: Actualiza los cálculos de DIFF en join_with_pivot y en
las ubicaciones equivalentes indicadas para restar h1 y h2 como enteros de mayor
rango, y solo después convertir el resultado al tipo de DIFF. Evita convertir
cada altura a signed char antes de la resta, preservando el valor correcto para
alturas superiores a 127.
- Around line 780-1214: Elimina las rutinas auxiliares sin llamadores visibles
—extract_min, extract_max, join_exclusive_rec, join_with_pivot, join_right,
join_left, split_key_rec, split_key_dup_rec y split_pos_rec— o intégralas en las
implementaciones públicas actuales de join/split. Si las conectas, sustituye los
recorridos iterativos y la reinserción por estas rutas O(log n), manteniendo la
semántica pública existente; no dejes dos algoritmos divergentes.
In `@tpl_binNodeUtils.H`:
- Around line 55-57: Elimina el calificador static de inorder_rec_helper y
conserva inline para evitar enlace interno innecesario en la plantilla. Aplica
el mismo cambio a todas las funciones de plantilla con el patrón inline static
indicadas en las líneas 100-102, 144-146, 799-801, 946-947, 1038, 1322 y 1776.
- Around line 2647-2659: El método reset_last() recorre todo el árbol mediante
size(root) para establecer pos. Elimina ese recuento eager, deja la posición
marcada como pendiente al reposicionar en advance_to_max(root) y haz que
get_pos() calcule el índice solo cuando sea necesario, manteniendo el resultado
correcto para el último elemento.
In `@tpl_dynArray.H`:
- Around line 59-60: Remove the duplicate `#include` <ah-errors.H> directive,
keeping a single inclusion in the header.
- Around line 1382-1385: Update has_curr() to avoid the signed/unsigned
comparison warning by preserving the curr_idx >= 0 guard and explicitly
converting curr_idx to size_t for the comparison with array_ptr->size(). Keep
the existing null-pointer and bounds behavior unchanged.
🪄 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: Pro Plus
Run ID: bb508895-0975-40b2-8242-e6f7cf146b2e
📒 Files selected for processing (14)
Tests/prefix_tree_test.ccTests/r_star_tree_test.ccTests/r_tree_debug_snapshot_test_helpers.HTests/r_tree_test.ccah-comb.Hah-ranges.Harray_it.Hpoint.Htikzgeom_algorithms.Htpl_array.Htpl_avl.Htpl_avlRk.Htpl_binNodeUtils.Htpl_dynArray.H
🚧 Files skipped from review as they are similar to previous changes (1)
- Tests/r_tree_debug_snapshot_test_helpers.H
Include tpl_array.H in r_tree_debug_snapshot_test_helpers.H to ensure proper compilation for Array<bool>.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
Examples/tikz_computational_geometry_gallery_example.cc:436
- This new example introduces
std::vector<Tikz_Plane>for panel aggregation. The project’s container policy strongly prefers Aleph containers when equivalents exist (hereArray<Tikz_Plane>works), and other parts of this PR are already migrating fromstd::vectortoArray. Consider switchingpanelstoArray<Tikz_Plane>and adjusting theemplace_back/backcall sites accordingly to keep examples consistent with the library’s container ecosystem.
std::vector<Tikz_Plane> panels;
panels.reserve(20);
auto & p = panels; // shorthand
Summary by CodeRabbit
debug_snapshot()en R-Tree y R*-Tree para inspeccionar la estructura.