Add gcov/lcov code coverage#66
Merged
MitchellThompkins merged 12 commits intomainfrom Apr 26, 2026
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🧰 Additional context used🧠 Learnings (25)📓 Common learnings📚 Learning: 2026-03-30T00:56:36.122ZApplied to files:
📚 Learning: 2026-03-14T04:45:38.194ZApplied to files:
📚 Learning: 2026-03-14T04:45:38.194ZApplied to files:
📚 Learning: 2026-03-30T00:56:36.122ZApplied to files:
📚 Learning: 2026-03-29T18:28:41.448ZApplied to files:
📚 Learning: 2026-03-30T00:56:36.122ZApplied to files:
📚 Learning: 2026-03-14T04:58:46.252ZApplied to files:
📚 Learning: 2026-03-30T00:56:36.122ZApplied to files:
📚 Learning: 2026-03-19T02:38:50.626ZApplied to files:
📚 Learning: 2026-04-26T06:18:55.803ZApplied to files:
📚 Learning: 2026-04-26T14:35:00.064ZApplied to files:
📚 Learning: 2026-03-23T06:44:11.621ZApplied to files:
📚 Learning: 2026-03-15T20:21:45.214ZApplied to files:
📚 Learning: 2026-04-26T13:50:51.296ZApplied to files:
📚 Learning: 2026-04-26T14:35:00.064ZApplied to files:
📚 Learning: 2026-03-15T20:21:39.128ZApplied to files:
📚 Learning: 2026-04-26T14:34:59.710ZApplied to files:
📚 Learning: 2026-03-23T06:44:08.436ZApplied to files:
📚 Learning: 2026-04-26T15:31:38.217ZApplied to files:
📚 Learning: 2026-03-21T04:39:24.399ZApplied to files:
📚 Learning: 2026-03-21T04:39:18.577ZApplied to files:
📚 Learning: 2026-03-26T00:30:56.738ZApplied to files:
📚 Learning: 2026-03-19T01:30:49.719ZApplied to files:
📚 Learning: 2026-03-16T01:24:22.804ZApplied to files:
🔇 Additional comments (3)
📝 WalkthroughWalkthroughAdds opt-in coverage instrumentation and reporting (CMake/Make/lcov/genhtml), a GitHub Actions Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant GH as GitHub Actions
participant Img as Container Image
participant Codecov as Codecov
Dev->>GH: push changes
GH->>GH: run build-and-publish-container
GH->>Img: build/publish image (produces image_tag)
Img-->>GH: expose image_tag
GH->>GH: on success -> start coverage job
GH->>Img: run coverage job using image_tag
Img->>Img: checkout repo (with submodules) and run `make coverage.gcc`
Img->>Img: build/tests run -> generate coverage.info
Img->>Img: lcov filters -> produce coverage_filtered.info and HTML
Img-->>GH: coverage_filtered.info available
GH->>Codecov: upload coverage_filtered.info (OIDC auth)
Codecov-->>GH: upload acknowledged
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 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)
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 |
MitchellThompkins
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/main.yml:
- Around line 186-201: The coverage job duplicates the GCC test run; update the
workflow so the coverage job (job name "coverage") depends on "build-and-test"
and does not re-run the GCC matrix; specifically, remove or skip the redundant
"make coverage.gcc" execution by either making "coverage" use needs:
build-and-test and only collect .gcda artifacts from the prior "build-and-test"
run, or collapse the gcc test matrix into the "coverage" job and replace "make
test.gcc" with "make coverage.gcc" there (adjust artifact upload/collection
steps accordingly) so "make test.gcc" is not executed twice.
- Around line 203-207: The Codecov upload step "Upload coverage reports to
Codecov" currently depends on secrets.CODECOV_TOKEN which is not available for
forked pull_requests; update the workflow by granting OIDC permissions (add
permissions: id-token: write and contents: read at the workflow or job level)
and change the Codecov step to use OIDC by setting use_oidc: true for the
"Upload coverage reports to Codecov" step (keeping files:
build-coverage/coverage_filtered.info), or if you must keep token-based uploads
add a conditional to that step to skip for fork PRs (use the
github.event.pull_request.head.repo.fork check or ensure the step only runs when
not a fork).
In `@CMakeLists.txt`:
- Around line 190-210: The coverage target currently (add_custom_target(coverage
...)) can produce stale or empty reports because it has no dependency on running
tests; update the coverage custom target to ensure tests run before capture by
either adding a DEPENDS on the test executables/targets or invoking ctest (e.g.,
a COMMAND ${CMAKE_CTEST_COMMAND} --output-on-failure) before the lcov --capture
step, and/or capture a baseline (zero) before running tests as part of the
recipe; alternatively, at minimum augment the COMMENT string to explicitly warn
"tests must be run first" so IDE users know the required ordering. Ensure you
modify the existing target named coverage and the recipe lines that call
${LCOV_EXECUTABLE}, ${GENHTML_EXECUTABLE}, and ${CMAKE_COMMAND} -E echo so the
new command ordering runs ctest (or depends on test targets) prior to lcov
--capture.
- Around line 187-188: The CMake call using find_program(... REQUIRED) for
LCOV_EXECUTABLE and GENHTML_EXECUTABLE is invalid for the declared minimum CMake
3.13; either bump cmake_minimum_required to 3.18 or remove the REQUIRED keyword
and add explicit checks: call find_program(LCOV_EXECUTABLE lcov) and
find_program(GENHTML_EXECUTABLE genhtml) and then test if either variable is
empty (if(NOT LCOV_EXECUTABLE OR NOT GENHTML_EXECUTABLE)) and emit a clear
message(FATAL_ERROR ...) or message(WARNING ...) instructing users to install
lcov/genhtml when CONSTEIG_COVERAGE is ON; adjust the logic that references
LCOV_EXECUTABLE/GENHTML_EXECUTABLE accordingly.
In `@Makefile`:
- Around line 421-423: The Makefile currently stops if ctest fails, preventing
the subsequent coverage target from running; update the test invocation so
failures don't abort the coverage generation by making the ctest invocation
tolerate non-zero exit (e.g., prefix the ctest command with a leading '-' or
append '|| true') while keeping the subsequent cmake --build
$(BUILD_PREFIX)-coverage --target coverage step unchanged; locate the ctest line
that runs "ctest --test-dir $(BUILD_PREFIX)-coverage -j$$(getconf
_NPROCESSORS_ONLN)" and modify it so it ignores ctest's exit status (or
alternatively split test execution and coverage-generation into separate Make
targets such as a dedicated build-and-test that keeps failing behavior and a
coverage-only target that always runs).
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 029ee224-a428-4201-9345-60cd21fe8e0a
📒 Files selected for processing (15)
.env.github/workflows/main.ymlCMakeLists.txtDockerfileMakefiledocs/getting-started.mddocs/guide/decompositions.mddocs/guide/eigensolvers.mddocs/guide/math-functions.mddocs/guide/matrix.mdinclude/consteig/math/math_backend.hppinclude/consteig/matrix/decompositions/hessenberg.hppinclude/consteig/matrix/decompositions/qr.hppinclude/consteig/matrix/matrix.hpptests/matrix/qr_eigen.test.cpp
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-publish-container
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{cpp,hpp,h}
📄 CodeRabbit inference engine (AGENT.md)
Always use braces for all control flow bodies (if, else, for, while), even single-line ones. No braceless statements.
Files:
include/consteig/math/math_backend.hppinclude/consteig/matrix/decompositions/hessenberg.hppinclude/consteig/matrix/decompositions/qr.hppinclude/consteig/matrix/matrix.hpptests/matrix/qr_eigen.test.cpp
include/consteig/**/*.{cpp,hpp}
📄 CodeRabbit inference engine (AGENT.md)
All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Files:
include/consteig/math/math_backend.hppinclude/consteig/matrix/decompositions/hessenberg.hppinclude/consteig/matrix/decompositions/qr.hppinclude/consteig/matrix/matrix.hpp
**/*.hpp
⚙️ CodeRabbit configuration file
**/*.hpp: This is a constexpr C++17 header-only library. Focus on:
- Correctness of constexpr computations and static_assert usage
- Type consistency (Matrix<float,...> must use float tolerances, Matrix<double,...> must use double)
- Explicit casts when indexing Eigen matrices with consteig::Size (unsigned → signed narrowing)
- Tolerance constants are defined in test_dependencies/test_tools.hpp — never hardcode tolerance values, and use the constant appropriate for the method type (direct, iterative, pathological, large magnitude)
- No standard library dependencies in core library code (freestanding constraint)
Files:
include/consteig/math/math_backend.hppinclude/consteig/matrix/decompositions/hessenberg.hppinclude/consteig/matrix/decompositions/qr.hppinclude/consteig/matrix/matrix.hpp
**/tests/**/*.{cpp,hpp}
📄 CodeRabbit inference engine (AGENT.md)
**/tests/**/*.{cpp,hpp}: Use explicit static_castEigen::Index(i) when indexing Eigen matrices with Size variables to satisfy strict build warnings about narrowing conversions.
Use CONSTEIG_TEST_TOLERANCE (double) for Matrix<double, ...> tolerance comparisons in static_assert and test assertions.
Use CONSTEIG_FLOAT_TEST_TOLERANCE (float) for Matrix<float, ...> tolerance comparisons in static_assert and test assertions.
Use PATHOLOGICAL_TOL for tests involving defective or ill-conditioned matrices to account for theoretical accuracy limitations.
Use LARGE_VAL_TOL when comparing results with large magnitude values.
Preserve the float vs double type distinction in test matrices. Do not change Matrix<float, ...> to Matrix<double, ...> or vice versa, as tests are intentionally typed for precision validation.
Use compile-time verification (static_assert with equalWithinMat) for testing matrix computations to ensure compile-time correctness.
Use runtime verification (ASSERT_TRUE/EXPECT_NEAR with equalWithinMat) when comparing Consteig results against Eigen library reference implementations.
Files:
tests/matrix/qr_eigen.test.cpp
**/tests/**
⚙️ CodeRabbit configuration file
**/tests/**: These are compile-time and runtime tests. Check:
- static_assert failures = build failures = test failures (treat equally)
- Tests are split into individual .cpp files intentionally to avoid constexpr budget exhaustion
- Do not suggest merging test files
- Tolerance constants should come from test_tools.hpp, never hardcoded
Files:
tests/matrix/qr_eigen.test.cpp
🧠 Learnings (43)
📚 Learning: 2026-03-23T05:47:32.320Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: Makefile:4-4
Timestamp: 2026-03-23T05:47:32.320Z
Learning: In this repository’s Makefile, do not flag the specific assignment line `CONTAINER_TAG := $(shell grep ^CONTAINER_TAG .env | cut -d= -f2)` as possibly producing an empty value. The repository includes a tracked `.env` file that is guaranteed to exist and to define `CONTAINER_TAG`, so the grep/cut extraction should succeed under normal circumstances.
Applied to files:
.env
📚 Learning: 2026-03-19T02:08:07.404Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Dockerfile:27-27
Timestamp: 2026-03-19T02:08:07.404Z
Learning: In MitchellThompkins/consteig (Dockerfile), package versions in `apk add` commands are intentionally left unpinned because this is a development image. The maintainer accepts that rebuilds will pull in newer package versions, and relies on previously published container images as a rollback mechanism if a new dependency breaks things. Do not flag unpinned `apk add` packages as a bug or suggest pinning.
Applied to files:
Dockerfile
📚 Learning: 2026-04-26T06:18:52.415Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/math/constmath.hpp:0-0
Timestamp: 2026-04-26T06:18:52.415Z
Learning: In MitchellThompkins/consteig, the project convention avoids adding explanatory comments when the code (including file/function naming) already communicates intent. Do not suggest adding descriptive comments for using-declarations, re-exports, or similar constructs where the purpose is evident from context and surrounding names (e.g., `math_backend.hpp`, `constmath.hpp`).
Applied to files:
docs/getting-started.md
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Consteig is a C++17 constexpr template library designed for compile-time computation of eigenvalues and eigenvectors of square matrices.
Applied to files:
docs/getting-started.mddocs/guide/matrix.mddocs/guide/math-functions.mddocs/guide/decompositions.md
📚 Learning: 2026-03-25T07:42:50.869Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 46
File: matrix/tests/matrix.test.cpp:442-445
Timestamp: 2026-03-25T07:42:50.869Z
Learning: In `matrix/tests/matrix.test.cpp` (MitchellThompkins/consteig), use `ASSERT_EQ` (not `EXPECT_NEAR`) for runtime checks in `make_matrix` tests. The function merely stores provided literal values into the matrix with no floating-point arithmetic, so the retrieved values are bit-for-bit identical to the inputs — exact equality is correct and `EXPECT_NEAR` with a tolerance constant is not needed.
Applied to files:
docs/getting-started.mdinclude/consteig/matrix/matrix.hppdocs/guide/matrix.mdtests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE (double) for Matrix<double, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
docs/getting-started.mdinclude/consteig/math/math_backend.hppinclude/consteig/matrix/matrix.hppdocs/guide/matrix.mdtests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Applied to files:
docs/getting-started.mddocs/guide/math-functions.md
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan`, `__builtin_nanf`, `__builtin_nanl`) are acceptable alternatives.
Applied to files:
docs/getting-started.mdinclude/consteig/math/math_backend.hppdocs/guide/math-functions.md
📚 Learning: 2026-03-28T08:54:06.975Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 45
File: docs/guide/matrix.md:133-135
Timestamp: 2026-03-28T08:54:06.975Z
Learning: In MitchellThompkins/consteig (`matrix/matrix.hpp`), the no-arg `isSymmetric()` overload uses `epsilon<T>()` (machine epsilon) as its tolerance — NOT `CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE`. `CONSTEIG_DEFAULT_SYMMETRIC_TOLERANCE` is the eigensolver's routing threshold, passed explicitly as an argument to the `isSymmetric(thresh)` overload inside `eig()` in `eigen/eigen.hpp`. Do not conflate the two.
Applied to files:
docs/getting-started.md
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to include/consteig/**/*.{cpp,hpp} : All core computations in the Consteig library must be constexpr and freestanding (no C++ standard library dependencies) in include/consteig/ headers.
Applied to files:
docs/getting-started.mdinclude/consteig/math/math_backend.hppCMakeLists.txtdocs/guide/math-functions.md
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use compile-time verification (static_assert with equalWithinMat) for testing matrix computations to ensure compile-time correctness.
Applied to files:
docs/getting-started.mdinclude/consteig/matrix/matrix.hppdocs/guide/matrix.mdtests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use CONSTEIG_FLOAT_TEST_TOLERANCE (float) for Matrix<float, ...> tolerance comparisons in static_assert and test assertions.
Applied to files:
docs/getting-started.mdinclude/consteig/matrix/matrix.hppdocs/guide/matrix.mdtests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: All computations can be done as static constexpr, enabling full compile-time evaluation.
Applied to files:
docs/getting-started.mddocs/guide/matrix.md
📚 Learning: 2026-03-14T04:45:25.802Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:25.802Z
Learning: In production headers under the math/, matrix/, or eigen/ directories, do not introduce or include standard library headers. This guideline prevents std dependency in these header-only/zero-dependency components. If you need NaN or similar constants, use compiler built-ins that are constexpr-compatible (e.g., __builtin_nan(""), __builtin_nanf(""), __builtin_nanl("")); note that the empty string argument is required for constexpr support, and non-empty strings like "nan" are not constant expressions. Apply this guidance to any production headers in these directories; tests may diverge and still use standard headers there.
Applied to files:
include/consteig/math/math_backend.hppinclude/consteig/matrix/decompositions/hessenberg.hppinclude/consteig/matrix/decompositions/qr.hppinclude/consteig/matrix/matrix.hpp
📚 Learning: 2026-04-26T06:18:52.415Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/math/constmath.hpp:0-0
Timestamp: 2026-04-26T06:18:52.415Z
Learning: In this codebase’s headers, follow the existing convention: avoid suggesting “explanatory” comments when intent is already clear from identifiers and context (including file/function naming). In particular, don’t recommend adding descriptive comments for using-declarations, re-exports, aliases, or similar constructs when their purpose is evident from surrounding code and names (e.g., cases like `math_backend.hpp`, `constmath.hpp`).
Applied to files:
include/consteig/math/math_backend.hppinclude/consteig/matrix/decompositions/hessenberg.hppinclude/consteig/matrix/decompositions/qr.hppinclude/consteig/matrix/matrix.hpp
📚 Learning: 2026-03-19T01:30:49.719Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Applied to files:
include/consteig/math/math_backend.hppMakefileCMakeLists.txtdocs/guide/math-functions.md
📚 Learning: 2026-03-14T04:52:20.667Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:52:20.667Z
Learning: In the consteig project (MitchellThompkins/consteig), there is no portable C++17, std-free, constexpr NaN that works across GCC, Clang, AND MSVC without including std headers. __builtin_nan("") is GCC/Clang-only and MSVC does not support it. The recommended approach is to remove `constexpr` from `nan()` in math/functions/utilities.hpp and use `static_cast<T>(0.0) / static_cast<T>(0.0)` as a runtime-only IEEE 754 NaN, ensuring no test evaluates sqrt(negative) at compile time. Preprocessor guards (`#if` defined(_MSC_VER)) can be used to separate MSVC and GCC/Clang paths if constexpr support on GCC/Clang is specifically needed.
Applied to files:
include/consteig/math/math_backend.hppdocs/guide/math-functions.md
📚 Learning: 2026-04-24T06:22:56.386Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/math/constmath.hpp:6-13
Timestamp: 2026-04-24T06:22:56.386Z
Learning: In MitchellThompkins/consteig, `CONSTEIG_USE_GCEM` and `CONSTEIG_GCEM_USE_STDLIB` are presence-based feature-enable macros: they are either defined (feature on) or not defined (feature off). `#ifdef`/`#ifndef` is the correct and idiomatic check for these macros. The `-DMACRO=0` usage pattern is non-standard for this project. Do not flag `#ifdef CONSTEIG_USE_GCEM` or `#ifndef CONSTEIG_GCEM_USE_STDLIB` as fragile or incorrect; they are intentional and correct.
Applied to files:
include/consteig/math/math_backend.hppdocs/guide/math-functions.md
📚 Learning: 2026-03-14T04:58:46.252Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:58:46.252Z
Learning: In the consteig project (MitchellThompkins/consteig), removing `constexpr` from `nan()` in math/functions/utilities.hpp does NOT solve the portability problem because `nan()` is called from `sqrt()` which is itself `constexpr`; calling a non-constexpr function from a constexpr function is ill-formed in C++17. The real architectural choices are: (1) use `__builtin_nan("")` with preprocessor guards if only GCC/Clang are targeted, or (2) make `sqrt(negative)` a hard compile-time error via static_assert and direct users to `csqrt()` for complex results when the sign of the input may be negative.
Applied to files:
include/consteig/math/math_backend.hppdocs/guide/math-functions.md
📚 Learning: 2026-03-29T19:26:42.088Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:39-39
Timestamp: 2026-03-29T19:26:42.088Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), `target_compile_features(consteig INTERFACE cxx_std_17)` is intentional and correct. It is a target-scoped enforcement: only targets that link against `consteig::consteig` are required to compile with C++17; it does NOT globally set `CMAKE_CXX_STANDARD` for the consumer's entire project. Do not flag this as conflicting with the `if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)` guard around `CMAKE_CXX_STANDARD`. The two serve different purposes: the guard-protected `CMAKE_CXX_STANDARD` drives the dev/test build; the INTERFACE feature requirement enforces the library's minimum language contract on consuming targets only.
Applied to files:
include/consteig/math/math_backend.hppCMakeLists.txt
📚 Learning: 2026-03-14T05:08:21.606Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T05:08:21.606Z
Learning: In math/functions/utilities.hpp of the consteig project, the NaN generation via static_cast<T>(0.0) / static_cast<T>(0.0) is an intentional design choice: it yields NaN at runtime on IEEE 754 platforms, but triggers a compile-time error (UB in constant expressions) in constexpr evaluation. Treat this as acceptable for a compile-time-first library: attempting to compute sqrt(negative) at compile time should be an error, and a separate constexpr path (csqrt()) should be used for negative inputs in constexpr contexts. There is no portable, std-free, ifdef-free, constexpr-NaN that works across GCC, Clang, and MSVC; this approach is the best given the project constraints. When reviewing, preserve this rationale and ensure any refactoring maintaining the same dual behavior, and document constraints about compiler support if modifying NaN generation mechanisms.
Applied to files:
include/consteig/math/math_backend.hpp
📚 Learning: 2026-04-26T06:18:55.803Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/math/constmath.hpp:18-33
Timestamp: 2026-04-26T06:18:55.803Z
Learning: In MitchellThompkins/consteig, do not recommend adding `noexcept` to math function declarations/overloads (including `constexpr` complex overloads like `abs(Complex<T>)` and `exp(Complex<T>)` in `include/consteig/math/constmath.hpp`). The project’s policy is that `noexcept` provides no meaningful guarantee for this constexpr-first freestanding library, and consistent omission is preferred; keep `noexcept` off these math APIs when reviewing.
Applied to files:
include/consteig/math/math_backend.hpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_TEST_TOLERANCE for double matrices in tests of iterative methods (Hessenberg, QR iteration).
Applied to files:
include/consteig/matrix/decompositions/hessenberg.hppinclude/consteig/matrix/decompositions/qr.hppinclude/consteig/matrix/matrix.hpptests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Preserve the float vs double type distinction in test matrices. Do not change Matrix<float, ...> to Matrix<double, ...> or vice versa, as tests are intentionally typed for precision validation.
Applied to files:
include/consteig/matrix/matrix.hppdocs/guide/matrix.mdtests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-15T20:21:39.128Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: .github/workflows/main.yml:31-31
Timestamp: 2026-03-15T20:21:39.128Z
Learning: In GitHub Actions workflows under .github/workflows, gating a build container job to run only on specific branches (e.g., develop/main) and using downstream jobs with needs and always() combined with a check for the upstream job result (e.g., needs.build_and_publish_container.result == 'success' or 'skipped') is an intentional pattern. It allows PRs to run against the latest published container without rebuilding on every PR. Do not flag this as a bug; this pattern can apply to similar workflows in this and other repos, and is technically auditable through the workflow syntax and needs conditions.
Applied to files:
.github/workflows/main.yml
📚 Learning: 2026-03-23T06:44:08.436Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: .github/workflows/main.yml:24-46
Timestamp: 2026-03-23T06:44:08.436Z
Learning: In .github/workflows/main.yml, it’s safe/intentional for the detect-changes step to hardcode `dockerfile_changed=false` and `env_changed=false` in the `else` branch when the workflow is not running for PR events. This is because direct pushes to `develop`/`main` are blocked via branch protection, so the guard logic only needs to reliably handle PR-triggered runs.
Applied to files:
.github/workflows/main.yml
📚 Learning: 2026-03-21T04:39:24.399Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
Applied to files:
MakefileCMakeLists.txt
📚 Learning: 2026-03-21T04:39:18.577Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Applied to files:
MakefileCMakeLists.txt
📚 Learning: 2026-03-29T18:28:41.448Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:121-143
Timestamp: 2026-03-29T18:28:41.448Z
Learning: In MitchellThompkins/consteig, MSVC (cl.exe / clang-cl) is NOT a supported compiler for dev builds (tests, examples, profiling). The `consteig_warnings` INTERFACE target in `CMakeLists.txt` intentionally uses only GCC/Clang-style flags (-Wall, -Wextra, -pedantic, -Werror, etc.) without MSVC guards. Do not flag the absence of MSVC-compatible equivalents (/W4, /WX) as an issue.
Applied to files:
MakefileCMakeLists.txtdocs/guide/math-functions.md
📚 Learning: 2026-03-26T00:30:56.738Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 47
File: Makefile:202-204
Timestamp: 2026-03-26T00:30:56.738Z
Learning: In MitchellThompkins/consteig, `profiling/run_profiling.sh` accepts an optional compiler argument that defaults to `g++` via `${1:-g++}`. An empty `$(CXX)` passed from the `profile` Makefile target is intentional and safe — it simply triggers the script's built-in default. Do not flag this as a missing validation or default.
Applied to files:
Makefile
📚 Learning: 2026-03-19T02:38:50.626Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: cmake/toolchains/arm-none-eabi-clang.cmake:13-14
Timestamp: 2026-03-19T02:38:50.626Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the library is and will always be a C++ header-only library (driven by its constexpr design goal). There is no C source compilation, so `CMAKE_C_FLAGS_INIT` in toolchain files (e.g., `cmake/toolchains/arm-none-eabi-clang.cmake`) does not need `-isystem` include paths for C++ or GCC headers. Do not suggest adding such paths to `CMAKE_C_FLAGS_INIT` for "completeness" or "future-proofing".
Applied to files:
MakefileCMakeLists.txt
📚 Learning: 2026-03-16T01:24:19.687Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: Makefile:196-197
Timestamp: 2026-03-16T01:24:19.687Z
Learning: In MitchellThompkins/consteig, the container.make.% recipe forwards CC=$(CC) and CXX=$(CXX) to the inner make call inside Docker. This behavior is intentional: the GNU Make defaults (cc, g++) align with the Alpine container defaults and are harmless. Do not treat this as a bug or flag it during reviews in this repo.
Applied to files:
Makefile
📚 Learning: 2026-03-29T18:43:47.241Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:0-0
Timestamp: 2026-03-29T18:43:47.241Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `project()` call explicitly declares `LANGUAGES CXX` (e.g., `project(consteig VERSION ${_consteig_version} LANGUAGES CXX)`). Since the library is header-only C++ with no C sources, this avoids an unnecessary C compiler probe at configure time. Do not flag the absence of a C language declaration as an issue.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-29T19:30:36.174Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:8-25
Timestamp: 2026-03-29T19:30:36.174Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `_consteig_version` fallback to `"0.0.0"` when Git or a tag is unavailable (shallow clones, source tarballs) is intentional. Do not suggest adding a dedicated `CONSTEIG_VERSION` cache/env override variable — packagers who need an explicit version can pass `-D_consteig_version=X.Y.Z` at configure time. This is a deliberate design choice, not an oversight.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-16T01:25:54.839Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: CMakeLists.txt:27-30
Timestamp: 2026-03-16T01:25:54.839Z
Learning: In CMakeLists.txt, keep C++ standard settings (CMAKE_CXX_STANDARD and CMAKE_CXX_EXTENSIONS OFF) inside the if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) guard. This ensures the consumer's C++ standard is not overridden when the project is added via add_subdirectory or FetchContent. Do not move these settings outside the guard; if a consumer does not opt into C++17+, their build should fail to compile due to C++17 syntax.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-29T17:49:00.041Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:171-180
Timestamp: 2026-03-29T17:49:00.041Z
Learning: For CMake projects, avoid hardcoding the project version in the `project()` call. Instead, derive the version at configure time from Git tags by running `git describe --tags --abbrev=0`, strip a leading `v` if present (e.g., `v1.2.3` -> `1.2.3`), and pass the resulting value to `project(<name> VERSION <derived_version> ...)`. Do not suggest adding a static `VERSION` argument to `project()` when the repository already intends to source it from Git tags.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-04-23T06:30:20.825Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 62
File: include/consteig/optional_dependencies/gcem/gcem_incl/tanh.hpp:74-92
Timestamp: 2026-04-23T06:30:20.825Z
Learning: In MitchellThompkins/consteig, all files under `include/consteig/optional_dependencies/gcem/` (including gcem_incl/ subdirectories) are vendored third-party dependency files (GCE-Math / gcem library by Keith O'Hara). These files must NOT be reviewed for code quality, style, or correctness issues. They are excluded via `!**/optional_dependencies/**` in .coderabbit.yaml. Any updates to these vendored files should come through a dedicated vendoring PR (e.g., via scripts/vendor_gcem.sh).
Applied to files:
docs/guide/math-functions.md
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/eigen/**/*.{cpp,hpp} : Use CONSTEIG_ITERATIVE_FLOAT_TOLERANCE for float matrices in tests of iterative methods (Hessenberg, QR iteration) to account for error accumulation.
Applied to files:
tests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use runtime verification (ASSERT_TRUE/EXPECT_NEAR with equalWithinMat) when comparing Consteig results against Eigen library reference implementations.
Applied to files:
tests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use explicit static_cast<Eigen::Index>(i) when indexing Eigen matrices with Size variables to satisfy strict build warnings about narrowing conversions.
Applied to files:
tests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use PATHOLOGICAL_TOL for tests involving defective or ill-conditioned matrices to account for theoretical accuracy limitations.
Applied to files:
tests/matrix/qr_eigen.test.cpp
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: The eigenvalue solver uses the Francis QR algorithm with balancing, Hessenberg reduction, QR iteration with implicit double-shift Wilkinson shifts, and deflation.
Applied to files:
docs/guide/decompositions.mddocs/guide/eigensolvers.md
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Matrix decompositions use Givens rotations for numerical stability.
Applied to files:
docs/guide/decompositions.md
🪛 checkmake (0.3.2)
Makefile
[warning] 414-414: Target body for "coverage.gcc" exceeds allowed length of 5 lines (9).
(maxbodylength)
🔇 Additional comments (15)
.env (1)
1-1:CONTAINER_TAGbump looks correct.Line 1 cleanly updates the container image tag used by CI and docker-compose; no issues found in this segment.
include/consteig/matrix/matrix.hpp (4)
424-424: Doc wording update is clear and accurate.This improves readability without changing behavior.
461-461: Implementation-detail note is appropriately phrased.Looks good; intent is concise and consistent with surrounding docs.
476-476:make_matrixfill-order description is clearer.No issues here; documentation aligns with implementation semantics.
488-488: Example caption update is good.Clean documentation-only improvement.
tests/matrix/qr_eigen.test.cpp (1)
68-69: LGTM – Improved comment clarity.The punctuation updates accurately clarify that null-space columns are non-unique in rank-deficient QR decompositions, explaining why
consteigand Eigen produce different valid results for those columns.include/consteig/math/math_backend.hpp (1)
16-16: Doc wording update is safe and clear.Line 16 is a non-functional comment edit; behavior and macro flow remain unchanged.
docs/getting-started.md (1)
145-145: Good docs-only refinement.Line 145 improves phrasing without changing the example semantics.
include/consteig/matrix/decompositions/hessenberg.hpp (1)
55-55: Doxygen update looks correct.Line 55 is documentation-only and preserves the template contract intent.
Dockerfile (1)
15-16: Coverage toolchain dependency add is appropriate.Lines 15–16 correctly extend the dev image with required utilities for the new coverage flow.
docs/guide/eigensolvers.md (1)
7-7: Heading normalization is clean and consistent.These line-level title formatting changes are documentation-only and safe.
Also applies to: 25-25, 37-37, 47-47
include/consteig/matrix/decompositions/qr.hpp (1)
23-24: QR result-field docs update is fine.Lines 23–24 are non-functional Doxygen punctuation refinements.
docs/guide/decompositions.md (1)
23-24: Documentation wording cleanup looks good.All touched lines are explanatory text refinements with no behavioral impact.
Also applies to: 41-42, 46-46, 70-73
docs/guide/math-functions.md (1)
23-23: Math guide text edits are safe.These are documentation-only phrasing updates and align with the rest of the guide.
Also applies to: 89-89, 96-96, 101-101
CMakeLists.txt (1)
178-184: No action needed for-fno-default-inline.GCC continues to support
-fno-default-inlinein modern versions. The flag is documented and functional, not deprecated or removed. It remains a valid way to control implicit inlining of class-defined member functions.> Likely an incorrect or invalid review comment.
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
Owner
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
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 @.github/workflows/main.yml:
- Around line 204-208: The Codecov upload step ("Upload coverage reports to
Codecov" using codecov/codecov-action@v5) currently may swallow failures because
fail_ci_if_error defaults to false; update the step to set the input
fail_ci_if_error: true so that the coverage job fails if the Codecov upload
fails (ensuring the files: build-coverage/coverage_filtered.info upload errors
surface as CI failures).
In `@CMakeLists.txt`:
- Line 169: The CONSTEIG_COVERAGE option is declared inside the
CONSTEIG_BUILD_TESTS block so -DCONSTEIG_COVERAGE=ON is ignored; move the
option(CONSTEIG_COVERAGE ...) out to the same scope as CONSTEIG_BUILD_TESTS so
the cache variable is always defined, and add a guard like if(CONSTEIG_COVERAGE
AND NOT CONSTEIG_BUILD_TESTS) message(WARNING "CONSTEIG_COVERAGE set but
CONSTEIG_BUILD_TESTS is OFF; coverage will not be applied") to warn users when
they enable coverage without enabling tests.
🪄 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: ASSERTIVE
Plan: Pro
Run ID: 21f7bb9e-2446-4c0c-b772-28715bbeb00a
📒 Files selected for processing (2)
.github/workflows/main.ymlCMakeLists.txt
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: clang-test-gcem-stdlib
- GitHub Check: clang-test
- GitHub Check: clang-test-gcem
🧰 Additional context used
🧠 Learnings (21)
📓 Common learnings
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 66
File: CMakeLists.txt:190-210
Timestamp: 2026-04-26T13:50:51.296Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `coverage` custom CMake target is intentionally CI-only. Coverage generation is driven exclusively via the `coverage.gcc` Makefile target (which enforces the correct configure → build → ctest → lcov capture order). Do not flag the absence of test dependencies or ctest invocation inside the `coverage` custom target as an issue; the ordering is enforced at the Makefile level.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 66
File: .github/workflows/main.yml:186-201
Timestamp: 2026-04-26T14:35:00.064Z
Learning: In MitchellThompkins/consteig (.github/workflows/main.yml), the `coverage` CI job is intentionally separate from the `build-and-test` matrix job. The coverage build requires `-O0` and `--coverage` (gcov/lcov) flags, which are incompatible with the optimized build used in `build-and-test`. Artifacts cannot be shared between the two runs, and replacing `test.gcc` with `coverage.gcc` in the matrix would drop the optimized test run entirely. The duplicate compute is an accepted cost of maintaining a separate instrumented build. Do not flag this as redundant or wasteful.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 66
File: Makefile:421-423
Timestamp: 2026-04-26T14:34:59.710Z
Learning: In MitchellThompkins/consteig (Makefile), the `coverage.gcc` target intentionally lets `ctest` exit non-zero and abort Make before the `coverage` target runs. Generating a coverage report on a failing build is considered misleading; coverage is only meaningful on a green build. The separate `build-and-test` CI job already surfaces test failures. Do not suggest prefixing `ctest` with `-` or `|| true` to ignore failures in this target.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
📚 Learning: 2026-04-26T14:35:00.064Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 66
File: .github/workflows/main.yml:186-201
Timestamp: 2026-04-26T14:35:00.064Z
Learning: In GitHub Actions workflows, if a separate CI job exists specifically to run an instrumented/coverage build (e.g., requires `-O0` plus `--coverage` for gcov/lcov) that cannot be combined with the optimized build/test job (e.g., `build-and-test` uses different flags), do not flag the duplication as redundant or wasteful. Treat it as intentional when the coverage toolchain settings are incompatible with the optimized build, artifacts cannot be shared across runs, and merging jobs (or swapping `test.gcc` for `coverage.gcc` in the matrix) would remove the optimized test coverage.
Applied to files:
.github/workflows/main.yml
📚 Learning: 2026-03-15T20:21:39.128Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: .github/workflows/main.yml:31-31
Timestamp: 2026-03-15T20:21:39.128Z
Learning: In GitHub Actions workflows under .github/workflows, gating a build container job to run only on specific branches (e.g., develop/main) and using downstream jobs with needs and always() combined with a check for the upstream job result (e.g., needs.build_and_publish_container.result == 'success' or 'skipped') is an intentional pattern. It allows PRs to run against the latest published container without rebuilding on every PR. Do not flag this as a bug; this pattern can apply to similar workflows in this and other repos, and is technically auditable through the workflow syntax and needs conditions.
Applied to files:
.github/workflows/main.yml
📚 Learning: 2026-04-26T13:50:51.296Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 66
File: CMakeLists.txt:190-210
Timestamp: 2026-04-26T13:50:51.296Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `coverage` custom CMake target is intentionally CI-only. Coverage generation is driven exclusively via the `coverage.gcc` Makefile target (which enforces the correct configure → build → ctest → lcov capture order). Do not flag the absence of test dependencies or ctest invocation inside the `coverage` custom target as an issue; the ordering is enforced at the Makefile level.
Applied to files:
.github/workflows/main.ymlCMakeLists.txt
📚 Learning: 2026-04-26T14:34:59.710Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 66
File: Makefile:421-423
Timestamp: 2026-04-26T14:34:59.710Z
Learning: In MitchellThompkins/consteig (Makefile), the `coverage.gcc` target intentionally lets `ctest` exit non-zero and abort Make before the `coverage` target runs. Generating a coverage report on a failing build is considered misleading; coverage is only meaningful on a green build. The separate `build-and-test` CI job already surfaces test failures. Do not suggest prefixing `ctest` with `-` or `|| true` to ignore failures in this target.
Applied to files:
.github/workflows/main.ymlCMakeLists.txt
📚 Learning: 2026-03-23T06:44:08.436Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 42
File: .github/workflows/main.yml:24-46
Timestamp: 2026-03-23T06:44:08.436Z
Learning: In .github/workflows/main.yml, it’s safe/intentional for the detect-changes step to hardcode `dockerfile_changed=false` and `env_changed=false` in the `else` branch when the workflow is not running for PR events. This is because direct pushes to `develop`/`main` are blocked via branch protection, so the guard logic only needs to reliably handle PR-triggered runs.
Applied to files:
.github/workflows/main.yml
📚 Learning: 2026-03-21T04:39:24.399Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:276-279
Timestamp: 2026-03-21T04:39:24.399Z
Learning: In MitchellThompkins/consteig (Makefile), the compiler-specific targets (`build.gcc`, `test.gcc`, `examples.gcc`, `build.clang`, `test.clang`, `examples.clang`, `cross.arm-gcc`, `cross.arm-clang`) intentionally do NOT use the top-level `CMAKE_OPTIONS` variable. They are self-contained replacements for the old `CC`/`CXX` env-var passthrough flow. They skip `INSTALL_PREFIX` (these targets are for build+test, not install) and `CLANG_TIDY_FIX` (clang-tidy is run via the default `make build` target, not via the compiler-specific targets). Do not flag this divergence as a bug.
Applied to files:
.github/workflows/main.ymlCMakeLists.txt
📚 Learning: 2026-03-21T04:39:18.577Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: Makefile:205-274
Timestamp: 2026-03-21T04:39:18.577Z
Learning: In MitchellThompkins/consteig (Makefile), CMAKE_OPTIONS is only populated when CC/CXX env vars are NOT set (the old env-var passthrough flow, lines ~22-28). The compiler-specific targets (build.gcc, build.clang, examples.gcc, examples.clang, cross.arm-gcc, cross.arm-clang) intentionally bypass CMAKE_OPTIONS because they are the replacement for that flow and set compilers/toolchains directly. Omitting INSTALL_PREFIX is fine (these are build+test targets, not install), and CLANG_TIDY_FIX is only used via the default `make build` target. Do not flag this as a divergence issue.
Applied to files:
.github/workflows/main.ymlCMakeLists.txt
📚 Learning: 2026-03-26T00:30:56.738Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 47
File: Makefile:202-204
Timestamp: 2026-03-26T00:30:56.738Z
Learning: In MitchellThompkins/consteig, `profiling/run_profiling.sh` accepts an optional compiler argument that defaults to `g++` via `${1:-g++}`. An empty `$(CXX)` passed from the `profile` Makefile target is intentional and safe — it simply triggers the script's built-in default. Do not flag this as a missing validation or default.
Applied to files:
.github/workflows/main.yml
📚 Learning: 2026-03-19T01:30:49.719Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: test_dependencies/gtest_stubs/gtest/gtest.h:7-24
Timestamp: 2026-03-19T01:30:49.719Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the gtest stubs at `test_dependencies/gtest_stubs/gtest/gtest.h` are used exclusively for cross-compilation builds (`CONSTEIG_COMPILE_ONLY=ON`). Tests that use `EXPECT_NEAR(...) << "message"` chaining (e.g., `eigen_comparison.test.cpp`, `eigenvectors.test.cpp`) are guarded by `if(NOT CONSTEIG_COMPILE_ONLY)` in their CMakeLists.txt and are never compiled against the stubs. The stubs only need to satisfy compilation for simpler tests that do not use `<<` message chaining, so the current no-op `((void)(x))` macro expansions are intentional and sufficient.
Applied to files:
.github/workflows/main.ymlCMakeLists.txt
📚 Learning: 2026-03-16T01:24:22.804Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: Makefile:196-197
Timestamp: 2026-03-16T01:24:22.804Z
Learning: In MitchellThompkins/consteig (Makefile), the `container.make.%` recipe intentionally always forwards `CC=$(CC)` and `CXX=$(CXX)` to the inner `make` call inside Docker, even when those aren't explicitly set by the caller. The GNU Make defaults (`cc`, `g++`) are equivalent to the Alpine container's defaults, so this is intentional and harmless. Do not flag this as a bug.
Applied to files:
.github/workflows/main.yml
📚 Learning: 2026-03-29T19:26:42.088Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:39-39
Timestamp: 2026-03-29T19:26:42.088Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), `target_compile_features(consteig INTERFACE cxx_std_17)` is intentional and correct. It is a target-scoped enforcement: only targets that link against `consteig::consteig` are required to compile with C++17; it does NOT globally set `CMAKE_CXX_STANDARD` for the consumer's entire project. Do not flag this as conflicting with the `if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME)` guard around `CMAKE_CXX_STANDARD`. The two serve different purposes: the guard-protected `CMAKE_CXX_STANDARD` drives the dev/test build; the INTERFACE feature requirement enforces the library's minimum language contract on consuming targets only.
Applied to files:
.github/workflows/main.ymlCMakeLists.txt
📚 Learning: 2026-04-26T14:35:00.064Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 66
File: .github/workflows/main.yml:186-201
Timestamp: 2026-04-26T14:35:00.064Z
Learning: In MitchellThompkins/consteig (.github/workflows/main.yml), the `coverage` CI job is intentionally separate from the `build-and-test` matrix job. The coverage build requires `-O0` and `--coverage` (gcov/lcov) flags, which are incompatible with the optimized build used in `build-and-test`. Artifacts cannot be shared between the two runs, and replacing `test.gcc` with `coverage.gcc` in the matrix would drop the optimized test run entirely. The duplicate compute is an accepted cost of maintaining a separate instrumented build. Do not flag this as redundant or wasteful.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-29T18:43:47.241Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:0-0
Timestamp: 2026-03-29T18:43:47.241Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `project()` call explicitly declares `LANGUAGES CXX` (e.g., `project(consteig VERSION ${_consteig_version} LANGUAGES CXX)`). Since the library is header-only C++ with no C sources, this avoids an unnecessary C compiler probe at configure time. Do not flag the absence of a C language declaration as an issue.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-29T19:30:36.174Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:8-25
Timestamp: 2026-03-29T19:30:36.174Z
Learning: In MitchellThompkins/consteig (CMakeLists.txt), the `_consteig_version` fallback to `"0.0.0"` when Git or a tag is unavailable (shallow clones, source tarballs) is intentional. Do not suggest adding a dedicated `CONSTEIG_VERSION` cache/env override variable — packagers who need an explicit version can pass `-D_consteig_version=X.Y.Z` at configure time. This is a deliberate design choice, not an oversight.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-29T18:28:41.448Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:121-143
Timestamp: 2026-03-29T18:28:41.448Z
Learning: In MitchellThompkins/consteig, MSVC (cl.exe / clang-cl) is NOT a supported compiler for dev builds (tests, examples, profiling). The `consteig_warnings` INTERFACE target in `CMakeLists.txt` intentionally uses only GCC/Clang-style flags (-Wall, -Wextra, -pedantic, -Werror, etc.) without MSVC guards. Do not flag the absence of MSVC-compatible equivalents (/W4, /WX) as an issue.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-29T17:49:00.041Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 51
File: CMakeLists.txt:171-180
Timestamp: 2026-03-29T17:49:00.041Z
Learning: For CMake projects, avoid hardcoding the project version in the `project()` call. Instead, derive the version at configure time from Git tags by running `git describe --tags --abbrev=0`, strip a leading `v` if present (e.g., `v1.2.3` -> `1.2.3`), and pass the resulting value to `project(<name> VERSION <derived_version> ...)`. Do not suggest adding a static `VERSION` argument to `project()` when the repository already intends to source it from Git tags.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-16T01:25:54.839Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 30
File: CMakeLists.txt:27-30
Timestamp: 2026-03-16T01:25:54.839Z
Learning: In CMakeLists.txt, keep C++ standard settings (CMAKE_CXX_STANDARD and CMAKE_CXX_EXTENSIONS OFF) inside the if(CMAKE_PROJECT_NAME STREQUAL PROJECT_NAME) guard. This ensures the consumer's C++ standard is not overridden when the project is added via add_subdirectory or FetchContent. Do not move these settings outside the guard; if a consumer does not opt into C++17+, their build should fail to compile due to C++17 syntax.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-30T00:56:36.122Z
Learnt from: CR
Repo: MitchellThompkins/consteig PR: 0
File: AGENT.md:0-0
Timestamp: 2026-03-30T00:56:36.122Z
Learning: Applies to **/tests/**/*.{cpp,hpp} : Use runtime verification (ASSERT_TRUE/EXPECT_NEAR with equalWithinMat) when comparing Consteig results against Eigen library reference implementations.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-14T04:45:38.194Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 25
File: math/functions/utilities.hpp:97-106
Timestamp: 2026-03-14T04:45:38.194Z
Learning: The project `consteig` (MitchellThompkins/consteig) explicitly avoids all `std` library usage outside of test files, as a zero-dependency design goal. Suggestions for production headers (under `math/`, `matrix/`, `eigen/`) must not introduce any `#include <...>` standard library headers. Compiler built-ins (e.g., `__builtin_nan("")`, `__builtin_nanf("")`, `__builtin_nanl("")`) are acceptable constexpr-compatible alternatives for generating NaN without std. The empty string argument `""` is required for constexpr compatibility — non-empty strings like `"nan"` are NOT constant expressions.
Applied to files:
CMakeLists.txt
📚 Learning: 2026-03-19T02:38:50.626Z
Learnt from: MitchellThompkins
Repo: MitchellThompkins/consteig PR: 31
File: cmake/toolchains/arm-none-eabi-clang.cmake:13-14
Timestamp: 2026-03-19T02:38:50.626Z
Learning: In the `consteig` project (MitchellThompkins/consteig), the library is and will always be a C++ header-only library (driven by its constexpr design goal). There is no C source compilation, so `CMAKE_C_FLAGS_INIT` in toolchain files (e.g., `cmake/toolchains/arm-none-eabi-clang.cmake`) does not need `-isystem` include paths for C++ or GCC headers. Do not suggest adding such paths to `CMAKE_C_FLAGS_INIT` for "completeness" or "future-proofing".
Applied to files:
CMakeLists.txt
🔇 Additional comments (1)
CMakeLists.txt (1)
151-153: The code does not mixtarget_link_librariessignatures. All test targets intests/matrix/CMakeLists.txt,tests/math/CMakeLists.txt, andtests/eigen/CMakeLists.txtconsistently use the plain signature (withoutPRIVATE/PUBLIC/INTERFACEkeywords) when linking, and thecreate_testmacro also uses the plain signature. Since all calls totarget_link_librariesfor a given test target are uniform, there is no CMake error. No change needed.
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
2 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.
Adds code coverage instrumentation via gcov/lcov, runnable inside the existing Docker container.
What changed
Dockerfile: installslcovviaapkCMakeLists.txt: addsCONSTEIG_COVERAGEoption; creates aconsteig_coverageinterface library with--coverage -O0 -fno-inline*flags; hooks into thecreate_testmacro so every test target is instrumented when the option is on; adds acoverageCMake target that runslcovcapture, filters out third-party paths (_deps/,/usr/, googletest), and generates an HTML report viagenhtmlMakefile: addscoverage.gcctarget that configures, builds, runs tests, and generates the report in one step; addsbuild-coverage/toremove; updateshelpUsage
The HTML report is written to
build-coverage/coverage_html/index.html.Note: the container image needs to be rebuilt (
make container.build) to pick uplcov.Summary by CodeRabbit
New Features
Chores
Documentation