feat: add LDLT factorization, citation metadata, and dev workflow updates #11
Conversation
…ates - Implement stack-allocated LDLT (Matrix::ldlt, Ldlt::det/solve_vec) for symmetric SPD/PSD matrices - Add DEFAULT_SINGULAR_TOL and keep DEFAULT_PIVOT_TOL as a compatibility alias - Add constructive proptests for LU/LDLT and extend matrix proptests - Add CITATION.cff and REFERENCES.md; link from README for separate citation support - Refresh just/CI/tooling configs (taplo, yamllint, cspell) and bump version to 0.1.2
📝 WalkthroughWalkthroughAdds a stack-allocated LDLᵀ factorization (Ldlt) and DEFAULT_SINGULAR_TOL, extends Matrix API and tests (property-based coverage), updates library exports and doctests, adds citation/reference metadata, and broad developer tooling/CI and linting enhancements. Changes
Sequence Diagram(s)(omitted) Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
Pre-merge checks✅ Passed checks (3 passed)
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 |
![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
Fix all issues with AI Agents 🤖
In @.gitignore:
- Around line 17-20: Remove the incorrect entries that ignore the package
manifest and the misnamed lockfile: delete the line containing "/package.json"
so package.json is tracked, and delete the line containing "/.package-lock.json"
(the dot-prefixed name is non-standard); keep the correct "package-lock.json"
entry (or "/package-lock.json") to continue ignoring the lockfile if intended.
In @justfile:
- Around line 293-400: The setup-tools recipe unconditionally exits when brew is
missing (the else branch that checks os and prints "❌ 'brew' not found."), which
prevents non-Darwin users from continuing despite the script already listing
system package manager instructions; remove the immediate exit in that else
branch and instead just warn (keep the Darwin-specific exit when os == "Darwin"
to prompt Homebrew install) so Linux/other systems can continue to the later
verification loop (refer to the os variable, the have() helper, and
install_with_brew and the missing-check loop) and let the final missing-tools
check fail the run if needed.
🧹 Nitpick comments (5)
REFERENCES.md (1)
13-30: Consider consolidating duplicate references.The same three bibliographic references appear in both the LU decomposition and LDLT sections. For better maintainability, you could create a single "References" section and cite them from both algorithm descriptions.
🔎 Proposed structure
### LU decomposition (Gaussian elimination with partial pivoting) The LU implementation in `la-stack` follows the standard Gaussian elimination / LU factorization -approach with partial pivoting for numerical stability. +approach with partial pivoting for numerical stability. See references [1-3] below. -- Golub, Gene H., and Charles F. Van Loan. *Matrix Computations* (4th ed.). Johns Hopkins University Press, 2013. -- Higham, Nicholas J. *Accuracy and Stability of Numerical Algorithms* (2nd ed.). SIAM, 2002. -- Trefethen, Lloyd N., and David Bau III. *Numerical Linear Algebra*. SIAM, 1997. - ### LDL^T factorization (symmetric SPD/PSD) The LDL^T (often abbreviated "LDLT") implementation in `la-stack` is intended for symmetric positive definite (SPD) and positive semi-definite (PSD) matrices (e.g. Gram matrices), and does not perform -pivoting. +pivoting. See references [1-3] below. +## References + +1. Golub, Gene H., and Charles F. Van Loan. *Matrix Computations* (4th ed.). Johns Hopkins University Press, 2013. +2. Higham, Nicholas J. *Accuracy and Stability of Numerical Algorithms* (2nd ed.). SIAM, 2002. +3. Trefethen, Lloyd N., and David Bau III. *Numerical Linear Algebra*. SIAM, 1997..github/workflows/ci.yml (3)
88-155: Consider more robust checksum pattern matching.The script assumes the checksum file uses exactly two spaces between the hash and filename (line 139). While this is common, it's fragile if the upstream format changes.
🔎 More robust grep pattern
- grep -F " $TARBALL" "$CHECKSUMS_FILE" > checksum.txt + grep -E "[[:space:]]+${TARBALL}$" "$CHECKSUMS_FILE" > checksum.txtThis pattern matches one or more whitespace characters (spaces or tabs) before the filename, making it more resilient to format variations.
157-185: Consider more robust checksum pattern matching.Similar to the actionlint installation, line 180 assumes exactly two spaces in the checksum file format.
🔎 More robust grep pattern
- grep -F " ${SHFMT_ASSET}" sha256sums.txt > checksum.txt + grep -E "[[:space:]]+${SHFMT_ASSET}$" sha256sums.txt > checksum.txtThis pattern is more resilient to whitespace variations in the upstream checksum file.
186-217: Consider more robust checksum pattern matching.Line 213 has the same pattern-matching fragility as the Linux version.
🔎 More robust grep pattern
- grep -F " ${SHFMT_ASSET}" sha256sums.txt > checksum.txt + grep -E "[[:space:]]+${SHFMT_ASSET}$" sha256sums.txt > checksum.txtjustfile (1)
519-534: Consider:yaml-fixusesprettierwithout ensuring it's installed.The
yaml-fixtarget depends on_ensure-npxbut usesnpx prettierwithout verifyingprettieris available. This will auto-install prettier via npx on first run (which is fine), but the behavior differs from other targets that check for tool presence first.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
.github/workflows/audit.yml.github/workflows/ci.yml.github/workflows/codacy.yml.github/workflows/codecov.yml.github/workflows/rust-clippy.yml.gitignore.taplo.toml.yamllintCITATION.cffCargo.tomlREADME.mdREFERENCES.mdWARP.mdcspell.jsonjustfilepyproject.tomlrust-toolchain.tomlsrc/ldlt.rssrc/lib.rssrc/lu.rssrc/matrix.rstests/proptest_factorizations.rstests/proptest_matrix.rs
🧰 Additional context used
🧬 Code graph analysis (5)
tests/proptest_matrix.rs (6)
tests/proptest_factorizations.rs (1)
small_f64(12-14)tests/proptest_vector.rs (1)
small_f64(9-11)src/matrix.rs (3)
from_rows(25-27)ldlt(195-197)det(215-217)src/ldlt.rs (2)
det(92-98)solve_vec(124-171)src/lu.rs (2)
det(168-174)solve_vec(109-149)src/vector.rs (2)
new(21-23)into_array(66-68)
src/matrix.rs (2)
src/ldlt.rs (1)
factor(30-74)src/lu.rs (1)
factor(19-83)
tests/proptest_factorizations.rs (4)
src/matrix.rs (4)
from_rows(25-27)ldlt(195-197)det(215-217)lu(160-162)src/ldlt.rs (2)
det(92-98)solve_vec(124-171)src/lu.rs (2)
det(168-174)solve_vec(109-149)src/vector.rs (2)
new(21-23)into_array(66-68)
src/ldlt.rs (2)
src/matrix.rs (4)
det(215-217)identity(57-67)ldlt(195-197)from_rows(25-27)src/vector.rs (2)
new(21-23)into_array(66-68)
src/lib.rs (1)
src/matrix.rs (2)
ldlt(195-197)lu(160-162)
🔇 Additional comments (53)
pyproject.toml (3)
1-45: LGTM!Build system configuration and project metadata are correctly structured. The license uses a valid SPDX identifier, and the setuptools configuration properly points to the
scripts/directory.
47-81: Well-documented Ruff configuration.The linting rules are comprehensive, and each ignore has a clear rationale documented inline. The per-file ignores for tests appropriately allow assertions and internal helper access.
89-95: LGTM!Pytest configuration follows best practices with
--strict-markersand--strict-configto catch configuration issues early. Test paths correctly point toscripts/tests..github/workflows/codecov.yml (2)
27-27: Excellent GitHub Actions security pinning.All GitHub Actions have been pinned to specific commit SHAs with version comments. This is a security best practice that prevents unexpected behavior from action updates and improves reproducibility.
Also applies to: 32-32, 37-37, 51-51, 79-79, 89-89
29-29: Inline comments clarify intent.Lines 29 and 34 add helpful inline comments ("Needed for Codecov diff analysis" and "toolchain/components are specified in rust-toolchain.toml") that improve maintainability of the CI configuration.
Also applies to: 34-34
Cargo.toml (2)
3-3: Version bump aligns with PR scope.The bump from 0.1.1 to 0.1.2 is appropriate for the introduction of LDLT factorization, citation metadata, and updated tooling.
11-12: Formatting changes are stylistic and safe.Array reflowing on categories/keywords and spacing adjustments to dev-dependency features are non-functional and do not impact behavior.
Also applies to: 19-20
.github/workflows/rust-clippy.yml (1)
9-9: LGTM! Formatting consistency improvements.The changes standardize YAML string quoting (single → double quotes) and inline comment formatting across the workflow file with no functional impact.
Also applies to: 27-27, 30-30, 32-32, 35-35, 61-61
.github/workflows/codacy.yml (1)
28-28: LGTM! Formatting consistency improvements.The changes standardize YAML string quoting (single → double quotes) with no functional impact on the Codacy security scanning workflow.
Also applies to: 50-50, 147-147
.yamllint (1)
1-18: LGTM! Well-configured YAML linting rules.The yamllint configuration appropriately:
- Extends the default profile
- Ignores Rust build artifacts (target directories)
- Sets reasonable line length (120 chars)
- Allows standard boolean representations
- Aligns with Prettier's comment spacing conventions
rust-toolchain.toml (2)
7-14: LGTM! Comment formatting improvements.The inline comments improve readability without changing the configuration.
Also applies to: 18-22
25-25: Theprofilefield is valid and supported.The
profile = "default"syntax is correct forrust-toolchain.toml. The profile field is supported under [toolchain] with valid values being "minimal", "default", and "complete". These values control which rustup components are installed. "default" is an appropriate choice for this configuration.cspell.json (1)
7-90: LGTM! Dictionary expansion aligns with PR scope.The new spell-check entries appropriately cover:
- LDLT factorization terminology (lines 34-36)
- Citation metadata authors: Golub, Higham, Trefethen (lines 27-28, 81)
- Citation infrastructure: orcid, Zenodo (lines 55, 90)
- Dev tooling: taplo, shfmt, samply, rustup (lines 69, 74-75, 78-79)
- Testing keywords: addopts, minversion, testpaths (lines 7, 42, 80)
- Technical terms: esac, submatrix, yerrorlines (lines 18, 76, 89)
All additions are justified by the PR's feature additions and workflow updates.
.taplo.toml (1)
1-20: LGTM! Well-configured Taplo setup.The configuration appropriately mirrors Cargo-style conservative formatting with clear comments explaining the philosophy. The settings are sensible for a Rust project.
.github/workflows/audit.yml (1)
31-36: LGTM! Good cache restoration strategy.Adding
restore-keys: advisory-db-enables the cache to restore from any branch's advisory database if an exact match isn't found, improving cache hit rates for the audit workflow..github/workflows/ci.yml (1)
82-86: LGTM! Appropriate tool installation.Adding taplo-cli support aligns well with the new .taplo.toml configuration for consistent TOML formatting across the project.
CITATION.cff (1)
12-12: ORCID URL is valid and resolves correctly.The provided ORCID URL (https://orcid.org/0000-0002-0797-0021) returns an HTTP 200 response and follows the standard ORCID format. No changes needed.
src/ldlt.rs (7)
21-26: LGTM! Well-structured factorization struct.The struct design is clean with appropriate derives. The
#[must_use]attribute ensures callers don't accidentally discard the factorization result.
28-74: LGTM! Robust LDLT factorization with comprehensive error handling.The implementation correctly:
- Uses symmetric rank-1 updates operating on the lower triangle only
- Checks for non-finite values at each critical step
- Treats
d <= tolas singular (appropriate for SPD/PSD where diagonal D must be positive)- Uses
mul_addfor numerical stability in the trailing submatrix update
44-46: Verify:d <= tolrejects valid positive semi-definite matrices with zero eigenvalues.The check
d <= tolwill reject PSD matrices with legitimately zero diagonal entries in D. This is intentional per the docs ("treats PSD degeneracy... as singular"), but users expecting to factorize rank-deficient Gram matrices may be surprised.Consider adding a note in the module-level docs clarifying that truly singular PSD matrices (rank < D) will fail factorization.
76-98: LGTM! Simple and correct determinant computation.The determinant is computed as the product of diagonal entries of D, which is correct for A = LDLᵀ since det(L) = 1.
124-171: LGTM! Correct three-phase solve with thorough error checking.The solve correctly implements:
- Forward substitution for L (unit diagonal)
- Diagonal solve for D
- Back substitution for Lᵀ
Each phase properly checks for non-finite values and the diagonal solve re-validates against
tol.
174-188: LGTM! Reasonable symmetry assertion for debug builds.The scaled epsilon approach (
1e-12 * scale) appropriately handles matrices of varying magnitudes.
190-336: LGTM! Comprehensive test coverage.Tests cover:
- Identity and diagonal SPD matrices across dimensions 2-5
- Known 2×2 and 3×3 SPD cases with verified solutions
- Degeneracy detection for rank-deficient matrices
- Non-finite input handling
justfile (3)
9-49: LGTM! Consistent tooling prerequisite checks.The
_ensure-*helper pattern provides clear, actionable error messages with installation guidance. Each helper follows the same structure, making the justfile predictable and maintainable.
102-114: LGTM! Clear CI simulation target.The
citarget properly orchestrates the full validation suite. The dependency chain (check bench-compile test-all examples) mirrors what CI would run.
217-224: LGTM! Well-organized lint groups.The separation into
lint-code,lint-config, andlint-docsprovides clear categorization and allows running targeted validation.src/lu.rs (1)
105-107: LGTM! Documentation clarification aligns with implementation.The updated wording correctly reflects that singularity detection uses the tolerance from factorization, not an exact zero check.
src/matrix.rs (3)
4-5: LGTM!Import added for the new
Ldlttype.
156-158: LGTM! Clearer singularity condition description.The updated documentation accurately describes the LU singularity check as checking the largest-magnitude candidate pivot.
164-197: LGTM! Well-documented public API for LDLT factorization.The documentation:
- Clearly states the intended use case (SPD/PSD matrices like Gram matrices)
- Provides a complete working example
- Documents error conditions accurately, including the subtle behavior that PSD degeneracy is treated as singular
tests/proptest_matrix.rs (2)
17-25: LGTM! Well-designed strategies for SPD matrix construction.The entry ranges are appropriately bounded:
- L entries in [-0.5, 0.5] keep the condition number reasonable
- Diagonal entries in [0.1, 2.0] are comfortably above typical tolerance values
112-169: LGTM! Thorough property test for LDLT correctness.The test:
- Constructs a guaranteed SPD matrix via A = LDLᵀ with known L and D
- Verifies determinant consistency between LDLT and LU paths
- Verifies solve correctness by checking against known true solution
The epsilon of 1e-8 is appropriate given the accumulated floating-point errors from matrix construction and factorization.
WARP.md (2)
15-32: LGTM! Updated developer guidance reflects new workflow.The commands are accurate and the addition of
just cifor pre-commit validation is helpful guidance.
38-42: LGTM! Code structure documentation updated.The addition of
DEFAULT_SINGULAR_TOLandsrc/ldlt.rsaccurately reflects the new module structure.README.md (5)
25-26: LGTM!Clear introduction of the new LDLT type with appropriate context (symmetric SPD/PSD).
84-92: LGTM! Clear LDLT usage example.The example demonstrates the LDLT API concisely with a symmetric SPD matrix and verifies the determinant.
99-101: LGTM! API table updated with LDLT entry.The table accurately reflects the new
Ldlt<D>type, its storage, purpose, and key methods.
122-126: LGTM! Streamlined contributor workflow.The updated commands (
setup,check,fix,ci) provide a clear, minimal workflow for contributors.
130-137: LGTM! Citation and references sections added.Good academic practice to include citation guidance and reference the algorithms used.
tests/proptest_factorizations.rs (7)
1-14: LGTM! Well-structured test module with good strategy design.The module documentation clearly explains the testing approach. The
small_f64()strategy generates values in [-100.0, 100.0] which provides good coverage without risking overflow in matrix operations.
16-29: Good strategy design for numerical stability.The factor entry ranges are appropriately bounded to avoid ill-conditioned matrices, and diagonal entries maintain safe margins from the singularity tolerance.
37-99: LDLT test construction is mathematically correct.The matrix construction properly builds A = L·D·L^T with unit-lower-triangular L. The loop bounds and symmetry assignments are correct, and the oracle for determinant (product of diagonal entries) aligns with LDLT theory.
101-179: LU no-permutation test is well-designed.The L·U construction is mathematically sound. Note that this test implicitly assumes LU factorization won't pivot significantly for these constructed matrices. Since L is unit-lower and U has the specified diagonal, the constructed A should generally not require pivoting, making the determinant oracle valid.
181-195: Row-swap test correctly validates permutation handling.The
prop_assume!($d >= 2)guard is technically redundant since the macro is only invoked with dimensions 2-5. However, it serves as good defensive documentation and won't impact test performance meaningfully since the condition is always true.
234-265: Correct determinant sign handling for permutation.The expected determinant correctly negates the product of U diagonals to account for the single row transposition. The solve verification follows the same pattern as other tests.
271-275: Comprehensive dimension coverage for the library's target use cases.Testing dimensions 2-5 aligns well with geometric/computational geometry applications. The 64-case proptest configuration provides good coverage without excessive test runtime.
src/lib.rs (6)
34-34: Module declaration added correctly.The
ldltmodule is properly declared alongside the existinglu,matrix, andvectormodules.
41-53: Well-designed tolerance constant refactoring.Introducing
DEFAULT_SINGULAR_TOLwithDEFAULT_PIVOT_TOLas a backward-compatible alias is a clean API evolution. The documentation clearly guides users on when to use each name.
59-61: Documentation improvement accurately covers both factorization types.The updated wording "pivot/diagonal" correctly reflects that
LaError::Singularcan arise from either LU pivot selection or LDLT diagonal degeneracy.
88-98: Prelude updates are complete and well-documented.The re-exports correctly expose
LdltandDEFAULT_SINGULAR_TOLalongside existing items. The prelude documentation accurately reflects the available exports.
107-111: Thorough constant validation tests.Using
epsilon = 0.0for exact equality is appropriate when testing that constants have their expected values. The alias assertion ensuresDEFAULT_PIVOT_TOLremains synchronized withDEFAULT_SINGULAR_TOL.
142-144: Good prelude smoke test extension.Testing both LU and LDLT paths in the prelude test ensures all re-exported items are accessible and functional. Using
DEFAULT_PIVOT_TOLfor LU andDEFAULT_SINGULAR_TOLfor LDLT demonstrates the intended usage pattern.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #11 +/- ##
==========================================
- Coverage 94.95% 92.06% -2.90%
==========================================
Files 3 4 +1
Lines 119 189 +70
==========================================
+ Hits 113 174 +61
- Misses 6 15 +9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Replaces `grep -F` with `awk` to reliably filter checksum files in CI workflows by matching the last field against the target filename. This ensures that the correct checksum is selected, even when the filename contains spaces or special characters. Also, updates the `yaml-fix` recipe in the justfile to use `prettier` if available, falling back to `npx` if not. This resolves an issue where YAML formatting would fail if `prettier` was not globally installed. Adds more terms to the spellcheck dictionary and a reference file. Refs: feat/ldlt
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @REFERENCES.md:
- Around line 22-24: Remove the trailing extra space before "definite" in the
sentence "The LDL^T (often abbreviated "LDLT") implementation in `la-stack` is
intended for symmetric positive definite (SPD) and positive semi-definite (PSD)
matrices (e.g. Gram matrices), and does not perform pivoting." — edit the
REFERENCES.md text so it reads "symmetric positive definite" (single space) to
eliminate the trailing whitespace.
🧹 Nitpick comments (1)
justfile (1)
519-551: Consider adding_ensure-npxdependency toyaml-fix.The
yaml-fixtarget usesprettieror falls back tonpx, but unlike other targets that use npx (e.g.,markdown-check,markdown-fix), it doesn't have_ensure-npxas a dependency. While the script handles the missing case with an error message, adding the dependency would be consistent with other targets.🔎 Proposed fix
# YAML -yaml-fix: +yaml-fix: _ensure-npx #!/usr/bin/env bash
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
.github/workflows/ci.yml.github/workflows/codecov.yml.gitignoreREADME.mdREFERENCES.mdcspell.jsonjustfilesrc/lib.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- .github/workflows/codecov.yml
- .gitignore
- cspell.json
🧰 Additional context used
🧬 Code graph analysis (1)
src/lib.rs (1)
src/matrix.rs (2)
ldlt(195-197)lu(160-162)
🪛 LanguageTool
REFERENCES.md
[uncategorized] ~38-~38: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... Mathematik* 16 (1970/71): 360–361. Full text 3. Huang...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~48-~48: Ensure spelling is correct
Context: ...ns-lineaires) 5. Brezinski, Claude. "La methode de Cholesky." (2005). [PDF](https://...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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). (4)
- GitHub Check: Codacy Security Scan
- GitHub Check: build (windows-latest)
- GitHub Check: build (macos-latest)
- GitHub Check: Code Coverage
🔇 Additional comments (15)
justfile (4)
9-49: Well-structured tool detection helpers.The
_ensure-*helper targets follow a consistent pattern with clear error messages directing users tojust setup. The use ofcommand -vfor tool detection is portable and idiomatic.
50-62: LGTM - Consistent file discovery pattern.The
action-linttarget correctly uses null-terminated file discovery viagit ls-files -zand handles the empty case gracefully.
102-113: Clear separation of mutating vs non-mutating targets.The
check(non-mutating) andci(comprehensive validation) targets are well-documented and logically composed from smaller lint/test targets.
173-175: Good fix chain composition.The
fixtarget correctly chains mutating operations across different file types in a logical order.REFERENCES.md (1)
1-55: Well-organized references documentation.The file provides clear citation guidance and properly categorized bibliographic references for LU and LDLT algorithms. The French title "La méthode de Cholesky" on line 48 is the original paper title and is correctly preserved.
README.md (2)
84-103: Clear LDLT example with valid SPD matrix.The example uses a tridiagonal matrix that is symmetric positive-definite, making it an appropriate demonstration of LDLT factorization without pivoting. The relationship to Cholesky decomposition is well-explained.
141-148: Good addition of citation and references sections.The citation guidance and references link provide proper attribution pathways for academic users.
.github/workflows/ci.yml (4)
25-29: Good practice: version pinning via environment variables.Centralizing version numbers makes updates easier and ensures consistency across installation steps.
88-155: Solid supply-chain hardening for actionlint installation.The SHA256 checksum verification against upstream checksums file is a good security practice. The temp directory with cleanup trap and proper error handling are well-implemented.
157-184: Consistent checksum verification for shfmt (Linux).The installation follows the same secure pattern as actionlint with SHA256 verification.
186-217: macOS shfmt installation mirrors Linux approach.The architecture detection (
arm64vsamd64) and checksum verification usingshasum -a 256(macOS equivalent) is correctly implemented.src/lib.rs (4)
58-70: Clean tolerance constant design with backward compatibility.Introducing
DEFAULT_SINGULAR_TOLas the primary constant while keepingDEFAULT_PIVOT_TOLas an alias maintains backward compatibility. The documentation clearly explains the semantic difference and preference.
32-48: Good doctest coverage for LDLT functionality.The doctest demonstrates the expected usage pattern with an SPD matrix and validates the determinant result with appropriate tolerance.
105-116: Prelude correctly updated with new exports.The prelude now exposes
LdltandDEFAULT_SINGULAR_TOLalongside existing items, maintaining ergonomic usage patterns.
125-128: Comprehensive test coverage for new constants and LDLT path.Tests validate both the tolerance values and the full LDLT workflow through the prelude, ensuring the public API works as documented.
Also applies to: 152-161
Adds a check for the `prettier` command to the `yaml-fix` recipe. If `prettier` isn't found, it falls back to `npx` to ensure YAML files are consistently formatted. This improves the development experience by ensuring consistent code style.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
REFERENCES.md (1)
48-48: Fix French spelling: "La méthode" requires an accent.Line 48 contains "La methode de Cholesky" but the proper French spelling is "La méthode" with an accent on the first 'e'. This is a minor documentation quality issue.
🔎 Proposed fix
-5. Brezinski, Claude. "La methode de Cholesky." (2005). +5. Brezinski, Claude. "La méthode de Cholesky." (2005).
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
REFERENCES.mdjustfile
🧰 Additional context used
🪛 LanguageTool
REFERENCES.md
[uncategorized] ~38-~38: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ... Mathematik* 16 (1970/71): 360–361. Full text 3. Huang...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~48-~48: Ensure spelling is correct
Context: ...ns-lineaires) 5. Brezinski, Claude. "La methode de Cholesky." (2005). [PDF](https://...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
⏰ 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). (5)
- GitHub Check: build (macos-latest)
- GitHub Check: build (ubuntu-latest)
- GitHub Check: build (windows-latest)
- GitHub Check: Code Coverage
- GitHub Check: Codacy Security Scan
🔇 Additional comments (14)
justfile (14)
9-61: Well-structured tool validation helpers.The
_ensure-*pattern provides consistent, fail-fast validation with helpful error messages directing users tojust setupor manual installation. The_ensure-prettier-or-npxfallback logic handles the common case where prettier may not be globally installed.
62-74: LGTM!Safe file handling with null-separated output and proper empty-list handling.
80-83: LGTM!Good practice to catch release-profile-only warnings that wouldn't surface in debug builds.
114-131: LGTM!The
cirecipe provides comprehensive validation matching CI expectations. Cleaning coverage artifacts alongside cargo clean is appropriate.
185-194: LGTM!The
fixrecipe provides a convenient one-stop command for applying all formatters.
229-236: LGTM!Well-organized lint groups with clear separation between code, documentation, and configuration linting.
238-266: LGTM!Consistent pattern with batched execution (
-n100) for performance.
288-303: LGTM!Good dependency chain ensuring
uv syncruns before type checking and linting.
305-412: LGTM!The setup-tools recipe now correctly handles non-Darwin systems by warning rather than immediately exiting, while still performing the final verification check. This addresses the previous review feedback.
413-442: LGTM!Proper use of
shellcheck -xfor following sourced files, and correct flags for check (-d) vs fix (-w) modes.
531-563: LGTM with minor observation.The dynamic
--yesflag detection for npx (lines 549-551) is good defensive coding for different npm versions. The fallback error handling at lines 553-557 is technically unreachable after_ensure-prettier-or-npxsucceeds, but serves as a safety net if the logic is ever modified.
477-515: LGTM!Consistent pattern with other recipes. Direct invocation with
"${files[@]}"is fine for the typically small number of TOML files in a Rust project.
565-577: LGTM!Using
--strictensures warnings are treated as errors, maintaining high quality standards for CI.
196-228: LGTM!Comprehensive and well-organized help output that aids developer onboarding.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores
Style
✏️ Tip: You can customize this high-level summary in your review settings.