healthchecks #55
healthchecks #55
Conversation
📝 WalkthroughWalkthroughA new Lua module for health checks has been introduced. This module manages system program availability by providing functions to check executability, register/unregister programs, and monitor their status. Additionally, several language server configuration files are updated to utilize this module by calling a registration function, ensuring that the health of various LSPs is tracked. Furthermore, the health module now includes the ability to register specific programs and language servers for health monitoring. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant HealthModule
participant SystemPath
participant VimHealth
User->>HealthModule: check()
HealthModule->>HealthModule: check_programs()
loop For each registered program
HealthModule->>SystemPath: in_path(program)
SystemPath-->>HealthModule: Boolean result
end
HealthModule->>VimHealth: Report status
sequenceDiagram
participant LSP_Config
participant HealthModule
LSP_Config->>HealthModule: register_lsp(lspName)
HealthModule-->>LSP_Config: Registration updated
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
plugins/nobbz/lua/nobbz/lsp/beancount.lua (1)
6-9:⚠️ Potential issueBug: journal_file is never assigned a value.
There appears to be an issue with the existing code: line 8 calculates a path but doesn't assign it to
journal_fileor any variable. This meansjournal_filewill always be nil, preventing both LSP setup and health registration.- vim.fs.joinpath(root_dir, "main.beancount") + journal_file = vim.fs.joinpath(root_dir, "main.beancount")
🧹 Nitpick comments (2)
plugins/nobbz/lua/nobbz/lsp/digestif.lua (1)
1-8: Consider consistent spacing across LSP configuration filesAll the reviewed LSP configuration files follow the same pattern but have slightly different spacing after the LSP setup. For consistency, you might want to standardize on either one empty line or no empty line before the health registration call.
plugins/nobbz/lua/nobbz/health.lua (1)
11-36: Well-organized health check reporting function.The function systematically categorizes and reports program status with appropriate severity levels. The sorting logic ensures critical issues are shown first.
However, consider using a table with named fields instead of positional indices for the
binariesentries to improve code readability:- table.insert(binaries, { program, vim.health.ok, "is installed", 1, }) + table.insert(binaries, { + name = program, + report = vim.health.ok, + message = "is installed", + priority = 1 + })Then update the sorting and reporting logic accordingly:
- if a[4] == b[4] then return a[1] < b[1] end + if a.priority == b.priority then return a.name < b.name end - return a[4] < b[4] + return a.priority < b.priority - info[2]("`" .. info[1] .. "` " .. info[3]) + info.report("`" .. info.name .. "` " .. info.message)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
plugins/nobbz/lua/nobbz/health.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/astro.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/beancount.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/digestif.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/elixir.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/gleam.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/html.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/lua.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/mdx.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/nil.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/nushell.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/oxide.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/python.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/rust.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/tailwind.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/typescript.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/zig.lua(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
plugins/nobbz/lua/nobbz/lsp/lua.lua (1)
Learnt from: NobbZ
PR: NobbZ/nobbz-vim#19
File: plugins/nobbz/lua/nobbz/lsp/beancount.lua:3-6
Timestamp: 2025-01-31T23:06:17.133Z
Learning: In the nobbz-vim project, `LSP_CAPAS` is defined as a global variable in `plugins/nobbz/lua/nobbz/init.lua` and should be used for LSP capabilities across the codebase.
🔇 Additional comments (22)
plugins/nobbz/lua/nobbz/lsp/astro.lua (1)
7-8: LGTM: Health check registration for astro LSP.The addition of the health registration for the "astro" language server is aligned with the PR's objective of implementing health checks across language servers.
plugins/nobbz/lua/nobbz/lsp/gleam.lua (1)
7-8: LGTM: Health check registration for gleam LSP.The addition of the health registration for the "gleam" language server follows the consistent pattern implemented across multiple language servers in this PR.
plugins/nobbz/lua/nobbz/lsp/zig.lua (1)
7-8: LGTM: Health check registration for zls.The addition of the health registration for the "zls" language server is consistent with the health check implementation pattern across the codebase.
plugins/nobbz/lua/nobbz/lsp/html.lua (1)
7-8: LGTM: Health check registration for html LSP.The addition of the health registration for the "html" language server follows the consistent pattern implemented across language servers in this PR.
plugins/nobbz/lua/nobbz/lsp/rust.lua (1)
56-56: Health monitoring integration looks goodThe addition of the health registration call for rust_analyzer is properly placed after the LSP configuration. This will help track the availability of the rust_analyzer executable in the system.
plugins/nobbz/lua/nobbz/lsp/nil.lua (1)
25-25: Health check registration looks goodThe addition of health monitoring for the nil_ls language server is appropriately placed after the LSP configuration.
plugins/nobbz/lua/nobbz/lsp/tailwind.lua (1)
8-8: Health check registration looks goodThe tailwindcss language server has been properly registered with the health module, allowing monitoring of its availability.
plugins/nobbz/lua/nobbz/lsp/digestif.lua (1)
8-8: Health check registration looks goodThe digestif language server has been properly registered with the health module for monitoring.
plugins/nobbz/lua/nobbz/lsp/typescript.lua (1)
7-8: LGTM: Health check registration added for TypeScript LSPThe new line correctly registers the TypeScript language server with the health monitoring system after the server setup.
plugins/nobbz/lua/nobbz/lsp/nushell.lua (1)
9-10: LGTM: Health check registration added for Nushell LSPThe new line correctly registers the Nushell language server with the health monitoring system after the server setup.
plugins/nobbz/lua/nobbz/lsp/mdx.lua (1)
13-14: LGTM: Health check registration added for MDX analyzer LSPThe new line correctly registers the MDX analyzer language server with the health monitoring system after the server setup.
plugins/nobbz/lua/nobbz/lsp/oxide.lua (1)
14-15: LGTM: Health check registration added for Markdown Oxide LSPThe new line correctly registers the Markdown Oxide language server with the health monitoring system after the server setup.
plugins/nobbz/lua/nobbz/lsp/python.lua (1)
18-18: Clean integration of health monitoring for the basedpyright LSP.The addition properly registers the basedpyright LSP with the health module without modifying existing functionality. This follows good practices by separating health monitoring concerns from the LSP configuration.
plugins/nobbz/lua/nobbz/lsp/elixir.lua (1)
9-9: Clean integration of health monitoring for the elixirls LSP.The addition properly registers the elixirls LSP with the health module without modifying existing functionality. This follows a consistent pattern across the LSP configuration files.
plugins/nobbz/lua/nobbz/lsp/lua.lua (1)
52-52: Clean integration of health monitoring for the lua_ls LSP.The addition properly registers the lua_ls LSP with the health module without modifying the existing complex setup. This maintains consistency with the health registration pattern seen in other LSP configuration files.
plugins/nobbz/lua/nobbz/lsp/beancount.lua (1)
19-19: Clean integration of health monitoring for the beancount LSP.The addition properly registers the beancount LSP with the health module and correctly places it within the conditional block, ensuring it only runs when the LSP is actually set up.
plugins/nobbz/lua/nobbz/health.lua (6)
1-4: Well-structured module initialization.The module setup with a clean separation between the public interface
Mand private stateprogramsis a good practice for maintainable Lua modules.
5-9: Thein_pathfunction is correctly implemented.The function properly uses
vim.fn.executableto check if a program is available in the system PATH.
38-48: Public function well-documented and implemented.The function documentation clearly explains its purpose and behavior when registering a program multiple times.
50-54: Unregister function is straightforward and well-documented.The function correctly removes a program from the health check registry.
71-73: Simple wrapper function is appropriate.The public
checkfunction provides a clean interface to the privatecheck_programsfunction.
75-75: Standard module return.Correctly returns the module table to expose the public API.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugins/nobbz/lua/nobbz/health.lua (1)
57-70: Add error handling for LSP configuration access.The LSP registration function should handle potential errors when accessing the LSP configuration. Currently, it assumes the LSP configuration exists and has the expected structure.
I see there's already an issue (#56) created to address this concern, so I'm marking this as a duplicate of the previous review comment.
🧹 Nitpick comments (2)
plugins/nobbz/lua/nobbz/health.lua (2)
12-37: Replace magic numbers with named constants.The criticality levels (1, 2, 3) are used as magic numbers in the code. Consider using named constants to improve readability and maintainability.
- for program, required in pairs(programs) do - if in_path(program) then - table.insert(binaries, { program, vim.health.ok, "is installed", 1, }) - elseif required then - table.insert(binaries, { program, vim.health.error, "is not installed", 3, }) - else - table.insert(binaries, { program, vim.health.warn, "is not installed", 2, }) - end - end + local CRITICALITY = { + OK = 1, + WARNING = 2, + ERROR = 3 + } + + for program, required in pairs(programs) do + if in_path(program) then + table.insert(binaries, { program, vim.health.ok, "is installed", CRITICALITY.OK, }) + elseif required then + table.insert(binaries, { program, vim.health.error, "is not installed", CRITICALITY.ERROR, }) + else + table.insert(binaries, { program, vim.health.warn, "is not installed", CRITICALITY.WARNING, }) + end + end
1-88: Consider adding a function to check a specific program.The module provides functions to register and unregister programs, but no way to check if a specific program is available without running the full health check. Consider adding a public function to check a specific program's availability.
+ ---Checks if a specific program is available in the PATH + ---@param program string the program to check + ---@return boolean whether the program is in the PATH + M.is_available = function(program) + return in_path(program) + end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
plugins/nobbz/lua/nobbz/health.lua(1 hunks)plugins/nobbz/lua/nobbz/init.lua(1 hunks)plugins/nobbz/lua/nobbz/lsp/beancount.lua(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- plugins/nobbz/lua/nobbz/lsp/beancount.lua
🔇 Additional comments (6)
plugins/nobbz/lua/nobbz/init.lua (1)
65-65: LGTM: Health module integration.The added line appropriately marks the health module as "done" after the lazy loading is finished, enabling proper reporting of configuration completion status in health checks.
plugins/nobbz/lua/nobbz/health.lua (5)
1-5: Good module initialization pattern.The module is correctly initialized with a local table
Mfor exports and appropriate local variables to track state. This follows Lua best practices for module design.
8-10: Well-implemented executable checker.The
in_pathfunction is a clean implementation that properly uses Neovim's built-invim.fn.executablefunction to check if a program exists in the PATH.
39-55: Well-documented API functions.The
register_programandunregister_programfunctions are well-documented with clear descriptions of parameters and behavior. Good practice!
72-74: Simple and effective state management.The
donefunction is a straightforward implementation for marking configuration completion.
76-85: Well-structured health check implementation.The
checkfunction properly reports on both configuration status and registered programs. The organization with a separatecheck_programsfunction is clean and maintainable.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugins/nobbz/lua/nobbz/health.lua (1)
67-80: Add error handling for LSP configuration access.The LSP registration function should handle potential errors when accessing the LSP configuration. Currently, it assumes the LSP configuration exists and has the expected structure.
I see this issue has already been identified and a follow-up issue (#56) has been created to address it later.
🧹 Nitpick comments (2)
plugins/nobbz/lua/nobbz/health.lua (2)
74-79: Consider using buffer-local autocmd for FileType eventThe autocmd created for upgrading program requirements is currently global. For better encapsulation, consider making it buffer-local by adding the
buffer = 0option.vim.api.nvim_create_autocmd("FileType", { pattern = pattern, + buffer = 0, callback = function() M.register_program(program, true) end, })
31-41: Consider using named table fields for clarityThe current implementation uses numeric indices in the
binariestable, which reduces code readability. Consider using named fields for better clarity:- table.insert(binaries, { program, vim.health.ok, "is installed", CRITICALITY.OK, }) + table.insert(binaries, { + name = program, + reporter = vim.health.ok, + message = "is installed", + criticality = CRITICALITY.OK + }) -- Update sort function and reporting accordingly table.sort(binaries, function(a, b) - if a[4] == b[4] then return a[1] < b[1] end + if a.criticality == b.criticality then return a.name < b.name end - return a[4] < b[4] + return a.criticality < b.criticality end) for _, info in ipairs(binaries) do - info[2]("`" .. info[1] .. "` " .. info[3]) + info.reporter("`" .. info.name .. "` " .. info.message) end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/nobbz/lua/nobbz/health.lua(1 hunks)
🔇 Additional comments (2)
plugins/nobbz/lua/nobbz/health.lua (2)
1-101: Well-structured health check module with clear responsibilitiesThis module provides a clean API for health checking system programs with good separation between public and private functions. The documentation is thorough and the implementation follows Lua module best practices.
31-37: Good sorting implementation for health check resultsThe sorting function correctly prioritizes criticality first and then sorts alphabetically by program name within each criticality level. This will provide a well-organized health check report.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
plugins/nobbz/lua/nobbz/health.lua (1)
69-86: Missing error handling in LSP registrationThe
register_lspfunction needs error handling when accessing the LSP configuration. It currently assumes the configuration exists and has the expected structure, which could lead to runtime errors.I notice from the past review comments that this issue has already been identified and a follow-up issue (#56) has been created to address it.
🧹 Nitpick comments (2)
plugins/nobbz/lua/nobbz/health.lua (2)
6-7: Consider using an enum pattern for CRITICALITYThe CRITICALITY table is well-defined, but you could consider using a more idiomatic enum pattern.
-local CRITICALITY = { OK = 1, ERROR = 2, WARN = 3, NOTICE = 4, } +local CRITICALITY = { + OK = 1, + ERROR = 2, + WARN = 3, + NOTICE = 4, +}
15-48: Consider using named fields for better readabilityThe
check_programsfunction has a good implementation but uses numeric indices for thebinariestable entries, which makes the code less maintainable.local function check_programs() local binaries = {} vim.health.start("Programs in `PATH`:") for program, required in pairs(programs) do if in_path(program) then - table.insert(binaries, { program, vim.health.ok, "is installed", CRITICALITY.OK, }) + table.insert(binaries, { + name = program, + report = vim.health.ok, + message = "is installed", + criticality = CRITICALITY.OK, + }) elseif required then - table.insert(binaries, { program, vim.health.error, "is not installed", CRITICALITY.ERROR, }) + table.insert(binaries, { + name = program, + report = vim.health.error, + message = "is not installed", + criticality = CRITICALITY.ERROR, + }) else - table.insert(binaries, { program, vim.health.info, "is not installed", CRITICALITY.NOTICE, }) + table.insert(binaries, { + name = program, + report = vim.health.info, + message = "is not installed", + criticality = CRITICALITY.NOTICE, + }) end end table.sort(binaries, function(a, b) -- Sort by program name, if same criticality - if a[4] == b[4] then return a[1] < b[1] end + if a.criticality == b.criticality then return a.name < b.name end -- Sort by criticality otherwise - return a[4] < b[4] + return a.criticality < b.criticality end) for _, info in ipairs(binaries) do - local report_func = info[2] - local program = vim.fs.basename(info[1]) - local msg = info[3] + local report_func = info.report + local program = vim.fs.basename(info.name) + local msg = info.message local message = string.format("`%s` %s", program, msg) report_func(message) end end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/nobbz/lua/nobbz/health.lua(1 hunks)
🔇 Additional comments (7)
plugins/nobbz/lua/nobbz/health.lua (7)
1-5: Well-structured module initializationGood module structure with clear separation between public and private components. The module table
Mis properly initialized along with the supporting data structures.
8-13: Good utility function with proper documentationThe
in_pathfunction is well-documented with proper JSDoc annotations and has a clean implementation using Neovim's API.
50-60: Well-documented public API functionThe
register_programfunction has excellent documentation that explains the behavior clearly, including what happens when a program is registered multiple times.
62-67: Good program unregistration implementationThe
unregister_programfunction is well-documented and follows Lua's idiomatic way to remove entries from a table.
88-92: Simple and effective configuration completion markerThe
donefunction provides a clean way to mark when configuration is completely loaded, which affects the health check status message.
94-105: Well-implemented health check functionThe
checkfunction effectively reports the overall configuration status and triggers the program checks. Good use of Neovim's health reporting API.
1-107: Overall well-designed health check moduleThis module provides a robust system for checking the health of programs and language servers. The implementation is clean, well-documented, and follows good programming practices. The automatic marking of LSP programs as required when relevant files are opened is particularly smart.
The only significant issue is the lack of error handling in the
register_lspfunction, which is already being tracked in issue #56.
Summary by CodeRabbit