Skip to content

Add semantic project search quickfix command#12

Closed
nhlmg93 wants to merge 1 commit into
pablopunk:mainfrom
nhlmg93:feature/pi-search
Closed

Add semantic project search quickfix command#12
nhlmg93 wants to merge 1 commit into
pablopunk:mainfrom
nhlmg93:feature/pi-search

Conversation

@nhlmg93

@nhlmg93 nhlmg93 commented May 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a semantic project search workflow to pi.nvim:

  • New :PiSearch command and require("pi").search() Lua API
  • Prompts the user for a natural-language project search query
  • Instructs pi to write quickfix-formatted search results to a temp file
  • Parses results into Neovim's quickfix list and opens it with :copen
  • Includes current buffer context when available
  • Documents usage and keymap example

Closes #11

Testing

make test

Summary by CodeRabbit

  • New Features

    • Added semantic search functionality accessible via the :PiSearch command or <leader>as keymap.
    • Search results are automatically populated in the quickfix list for easy navigation.
  • Documentation

    • Updated README with semantic search features, commands, keymaps, and usage examples.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 10, 2026

Copy link
Copy Markdown

Walkthrough

This PR implements semantic project search for pi.nvim. It adds a dedicated search prompt format that instructs the model to return only quickfix-style file locations (/path:line:col,line_count,notes), a new context builder for search queries, session lifecycle updates to skip buffer reloads, result parsing from a temp file into quickfix items, and the :PiSearch command for interactive search. Documentation and test coverage validate the feature end-to-end.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: adding a semantic project search feature with quickfix integration.
Linked Issues check ✅ Passed All coding requirements from issue #11 are met: semantic search prompting, structured location output format, quickfix list population, and :PiSearch command/API.
Out of Scope Changes check ✅ Passed All changes align with issue #11 objectives; no unrelated modifications detected across documentation, context handling, core API, plugin commands, and tests.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@lua/pi/context.lua`:
- Line 116: The gsub call on cleaned_message uses the pattern "@?" which treats
? as a pattern quantifier; update the pattern to escape the question mark so it
matches the literal sequence by changing the call on cleaned_message (the gsub
invocation) to use "@%?" (e.g., cleaned_message = cleaned_message:gsub("@%?",
"") ) so both characters are matched literally.
- Line 114: The pattern check using cleaned_message:find("@?") treats '?' as a
pattern quantifier, so update the call to perform literal matching of the
sequence "@?" (either by using plain-text matching with the third-argument true
on the find call or by escaping the question mark in the pattern), replacing the
existing cleaned_message:find("@?") usage in the code around the cleaned_message
handling.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6fc55590-e4ed-4396-98c6-594de99a9c40

📥 Commits

Reviewing files that changed from the base of the PR and between c56f7f9 and 9abadcf.

📒 Files selected for processing (6)
  • README.md
  • lua/pi/context.lua
  • lua/pi/init.lua
  • lua/pi/runner.lua
  • plugin/pi.nvim.lua
  • tests/test_pi_commands.lua

Comment thread lua/pi/context.lua Outdated
local cleaned_message = message

-- Check for @? directive
if cleaned_message:find("@?") then

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Fix the pattern matching for the @? directive.

Line 114 uses find("@?") which treats ? as a Lua pattern quantifier (0 or 1 of previous char), not a literal character. This means it would match just @ alone, which is incorrect.

🔧 Proposed fix to use literal string matching
-  if cleaned_message:find("@?") then
+  if cleaned_message:find("@?", 1, true) then

The third parameter true enables plain text matching, treating ? as a literal character.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if cleaned_message:find("@?") then
if cleaned_message:find("@?", 1, true) then
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/pi/context.lua` at line 114, The pattern check using
cleaned_message:find("@?") treats '?' as a pattern quantifier, so update the
call to perform literal matching of the sequence "@?" (either by using
plain-text matching with the third-argument true on the find call or by escaping
the question mark in the pattern), replacing the existing
cleaned_message:find("@?") usage in the code around the cleaned_message
handling.

Comment thread lua/pi/context.lua Outdated
@nhlmg93 nhlmg93 force-pushed the feature/pi-search branch from 9abadcf to e648a27 Compare May 10, 2026 18:00
@nhlmg93 nhlmg93 force-pushed the feature/pi-search branch from e648a27 to b3bb88c Compare May 10, 2026 18:38

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
lua/pi/init.lua (1)

106-128: ⚖️ Poor tradeoff

Consider a more robust parsing approach for the search result format.

The pattern ^(.-):(%d+):(.+)$ uses non-greedy matching from the start, which is fragile for filenames that might contain colons. While the documented format is /absolute/path/to/file:lnum:col,line_count,notes and the AI is instructed to output this specific format, a more robust approach would match from the right to find the last :digits: occurrence. This would be safer for edge cases and makes the intent clearer.

For example, instead of ^(.-):(%d+):(.+)$, consider matching backwards or using a pattern that explicitly handles the expected format more defensively (e.g., (.+):(%d+):(%d+,.*)$).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lua/pi/init.lua` around lines 106 - 128, The current parse_search_line uses a
non-greedy filename pattern which breaks when filenames contain colons; update
parse_search_line to locate the lnum/col section from the right by changing the
filename/line split to a greedy filename match (e.g., use "^(.*):(%d+):(.+)$"
instead of "^(.-):(%d+):(.+)$") so the last ":<digits>:" is used as the
separator, keep the subsequent parsing of rest (col,line_count,notes) as-is (the
local function parse_search_line and its call to normalize_search_line are the
loci to change) to ensure filenames with colons are handled robustly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@lua/pi/init.lua`:
- Around line 106-128: The current parse_search_line uses a non-greedy filename
pattern which breaks when filenames contain colons; update parse_search_line to
locate the lnum/col section from the right by changing the filename/line split
to a greedy filename match (e.g., use "^(.*):(%d+):(.+)$" instead of
"^(.-):(%d+):(.+)$") so the last ":<digits>:" is used as the separator, keep the
subsequent parsing of rest (col,line_count,notes) as-is (the local function
parse_search_line and its call to normalize_search_line are the loci to change)
to ensure filenames with colons are handled robustly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2edabedd-d52e-429d-b6c0-5e9c66f6c75a

📥 Commits

Reviewing files that changed from the base of the PR and between 9abadcf and b3bb88c.

📒 Files selected for processing (5)
  • README.md
  • lua/pi/context.lua
  • lua/pi/init.lua
  • plugin/pi.nvim.lua
  • tests/test_pi_commands.lua
✅ Files skipped from review due to trivial changes (2)
  • plugin/pi.nvim.lua
  • README.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/test_pi_commands.lua

@nhlmg93

nhlmg93 commented May 10, 2026

Copy link
Copy Markdown
Contributor Author

Closing in favor of a smaller core abstraction PR that lets downstream configs/plugins implement workflows like semantic search without adding them to pi.nvim core.

@nhlmg93 nhlmg93 closed this May 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add semantic project search that populates the quickfix list

1 participant