Phase 0 provider abstraction#1
Conversation
Reviewer's GuideIntroduces a new framework-agnostic provider architecture and renames the plugin from fastapi.nvim to nimbleapi.nvim, adds a Spring/Spring Boot Java provider with Tree-sitter queries and diagnostics, and refactors caches, explorer, pickers, codelens, and entrypoints to be provider-aware while updating documentation and adding a small headless test. Sequence diagram for :NimbleAPI info and provider detectionsequenceDiagram
actor User
participant Neovim as NeovimCommand
participant API as nimbleapi_init
participant Providers as providers_init
participant RouteProvider
User->>Neovim: :NimbleAPI info
Neovim->>API: info(ctx)
API->>Providers: info(ctx)
activate Providers
Providers->>Providers: resolve_root(ctx)
Providers-->>Providers: root
Providers->>Providers: get_provider(ctx)
alt provider cached
Providers-->>Providers: return cached provider
else not cached
Providers->>Providers: detect(root)
loop for each registered provider
Providers->>RouteProvider: check_prerequisites()
alt prerequisites ok
Providers->>RouteProvider: detect(root)
alt project matches
Providers-->>Providers: set active provider
else no match
Providers-->>Providers: record diagnostics
end
else prerequisites failed
Providers-->>Providers: record diagnostics
end
end
end
Providers-->>API: lines[] diagnostics and status
deactivate Providers
API-->>Neovim: lines[]
Neovim-->>User: show notification with provider info
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In
plugin/nimbleapi.luathe BufWritePost/BufEnter autocmd patterns are hard-coded to*.pyand*.java; consider deriving these fromrequire("nimbleapi.providers").get_all_extensions()so new providers automatically participate in file watching and codelens without touching the plugin layer. - The
RouteProvidertype inproviders/init.luadocumentsfind_project_rootasfun(): string, but it is always called with astartpathand the concrete implementations accept an argument; aligning the type annotation and call sites (e.g., consistentlyfind_project_root(startpath: string|nil)) would make the interface clearer.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `plugin/nimbleapi.lua` the BufWritePost/BufEnter autocmd patterns are hard-coded to `*.py` and `*.java`; consider deriving these from `require("nimbleapi.providers").get_all_extensions()` so new providers automatically participate in file watching and codelens without touching the plugin layer.
- The `RouteProvider` type in `providers/init.lua` documents `find_project_root` as `fun(): string`, but it is always called with a `startpath` and the concrete implementations accept an argument; aligning the type annotation and call sites (e.g., consistently `find_project_root(startpath: string|nil)`) would make the interface clearer.
## Individual Comments
### Comment 1
<location path="lua/nimbleapi/cache.lua" line_range="100-109" />
<code_context>
+function M.get_route_tree(ctx)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Avoid repeated notifications when no provider is detected by caching the empty result per root
When `get_route_tree` sees `get_provider` return `nil`, it warns but never sets `tree_cache[root]`. With `provider_cache[root] = false`, later calls still hit this branch and warn every time. Consider caching the empty result as `tree_cache[root] = false` before returning so callers can skip this path without repeating the warning.
</issue_to_address>
### Comment 2
<location path="lua/nimbleapi/cache.lua" line_range="146-150" />
<code_context>
+--- Get all routes as a flat list (with full paths).
+---@param ctx string|table|nil
+---@return table[]
+function M.get_all_routes(ctx)
+ local providers = require("nimbleapi.providers")
+ local root = providers.resolve_root(ctx)
+ if flat_cache[root] ~= nil then
+ return flat_cache[root] or {}
+ end
+
+ local provider = providers.get_provider(ctx)
+
+ if not provider then
+ -- Trigger get_route_tree so the user sees the diagnostic message
+ M.get_route_tree(ctx)
+ return {}
+ end
+
+ local project_root = provider.find_project_root(root)
+ local routes = provider.get_all_routes(project_root)
+ if not routes or #routes == 0 then
+ -- Try via route tree for providers that build trees
+ local tree = M.get_route_tree(ctx)
+ if tree then
+ routes = require("nimbleapi.router_resolver").flatten_routes(tree)
+ end
+ end
+ flat_cache[root] = routes or false
+ return routes or {}
+end
+
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Similarly cache the absence of a provider in flat route cache to prevent repeated diagnostics
When no provider is found, you call `get_route_tree(ctx)` to emit diagnostics but don’t update `flat_cache[root]`, so subsequent calls keep recomputing and re-emitting diagnostics. To keep behavior idempotent and consistent with `get_route_tree`, set `flat_cache[root] = false` before returning `{}` in this branch.
```suggestion
if not provider then
-- Trigger get_route_tree so the user sees the diagnostic message
M.get_route_tree(ctx)
flat_cache[root] = false
return {}
end
```
</issue_to_address>
### Comment 3
<location path="lua/nimbleapi/providers/init.lua" line_range="18" />
<code_context>
+---@field extract_routes fun(filepath: string): table[]
+---@field extract_includes fun(filepath: string): table[]
+---@field extract_test_calls_buf fun(bufnr: integer): table[]
+---@field find_project_root fun(): string
+```
+
</code_context>
<issue_to_address>
**nitpick:** Update the RouteProvider type annotation for find_project_root to reflect the startpath parameter
`find_project_root` is documented as taking no parameters, but both `fastapi` and `springboot` providers implement it as `find_project_root(startpath)` and it’s called as `provider.find_project_root(root)`. Please update the annotation to `---@field find_project_root fun(startpath: string|nil): string` so it matches actual usage and supports static tooling.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| function M.get_route_tree(ctx) | ||
| local providers = require("nimbleapi.providers") | ||
| local root = providers.resolve_root(ctx) | ||
| if tree_cache[root] ~= nil then | ||
| return tree_cache[root] or nil | ||
| end | ||
|
|
||
| local provider = providers.get_provider(ctx) | ||
|
|
||
| if not provider then |
There was a problem hiding this comment.
suggestion (bug_risk): Avoid repeated notifications when no provider is detected by caching the empty result per root
When get_route_tree sees get_provider return nil, it warns but never sets tree_cache[root]. With provider_cache[root] = false, later calls still hit this branch and warn every time. Consider caching the empty result as tree_cache[root] = false before returning so callers can skip this path without repeating the warning.
| if not provider then | ||
| -- Trigger get_route_tree so the user sees the diagnostic message | ||
| M.get_route_tree(ctx) | ||
| return {} | ||
| end |
There was a problem hiding this comment.
suggestion (bug_risk): Similarly cache the absence of a provider in flat route cache to prevent repeated diagnostics
When no provider is found, you call get_route_tree(ctx) to emit diagnostics but don’t update flat_cache[root], so subsequent calls keep recomputing and re-emitting diagnostics. To keep behavior idempotent and consistent with get_route_tree, set flat_cache[root] = false before returning {} in this branch.
| if not provider then | |
| -- Trigger get_route_tree so the user sees the diagnostic message | |
| M.get_route_tree(ctx) | |
| return {} | |
| end | |
| if not provider then | |
| -- Trigger get_route_tree so the user sees the diagnostic message | |
| M.get_route_tree(ctx) | |
| flat_cache[root] = false | |
| return {} | |
| end |
| ---@field extract_routes fun(filepath: string): table[] | ||
| ---@field extract_includes fun(filepath: string): table[] | ||
| ---@field extract_test_calls_buf fun(bufnr: integer): table[] | ||
| ---@field find_project_root fun(): string |
There was a problem hiding this comment.
nitpick: Update the RouteProvider type annotation for find_project_root to reflect the startpath parameter
find_project_root is documented as taking no parameters, but both fastapi and springboot providers implement it as find_project_root(startpath) and it’s called as provider.find_project_root(root). Please update the annotation to ---@field find_project_root fun(startpath: string|nil): string so it matches actual usage and supports static tooling.
Summary by Sourcery
Introduce a framework-agnostic NimbleAPI plugin with a pluggable provider system and migrate existing FastAPI-specific functionality to support both FastAPI (Python) and Spring Boot (Java) projects.
New Features:
Enhancements:
Tests: