| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Signed-off-by: Ertan <ertan.kucukoglu@gmail.com>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
Sorry, something went wrong.
|
Could a maintainer please rerun the failed test-windows-guards job? The failure occurred during setup in test_daemon_stability.py:
The preceding daemon lifecycle and stability sections passed. Several following guards then failed or skipped on daemon/index/UI startup preconditions. This PR only changes the Windows search_code PowerShell producer cap, so the daemon startup failure appears unrelated and potentially transient. |
Sorry, something went wrong.
|
Maintainer decision on this one, and it goes a different way — but the problem you identified is real and is going to get solved properly. We will not take a producer-side cap. The reason is result completeness. Our C-side cap counts accepted results: at src/mcp/mcp.c:9245 a line that fails the path_filter does continue without spending quota, so today PowerShell keeps streaming until we have 500 matching results. Select-Object -First 500 stops it at 500 unfiltered lines, and we then filter those down. On an unscoped search with a path_filter on Windows that silently returns somewhere between 0 and 500 where it previously returned 500 — and total_grep_matches reports the truncated count as though it were the whole answer. Two further consequences: the same query would return different results on Windows and POSIX, and it re-establishes the producer as the truncation authority, which the comment at mcp.c:9770 records as deliberately removed (the -m flag let large vendored files exhaust the quota before reaching project source). In this project a search that quietly returns fewer results than it found is a correctness bug, not a performance tradeoff. So a bound is right — a silent one is not. The direction instead: generic pagination for search_code, on every platform. The response carries a page indicator and a signal for whether more results exist, and a flag lets the caller page through. That solves what you are actually solving — bounding a pathological scan — without any platform behaving differently from another and without ever discarding a result the caller cannot know about. There is precedent in the codebase: tools/list is already cursor-paginated (MCP_TOOLS_PAGE_SIZE). So this PR will be closed in favour of that work, not because the diagnosis was wrong but because the bound belongs at a different layer. What I would genuinely like, if you are up for it: #1604 is being taken — it is results-preserving by construction and brings Windows to the parity POSIX already has via grep --include. And your #1606 is already merged; the static char buf in cbm_mkdtemp was a real concurrency defect where two daemon sessions could be handed the same temp directory, and it was the cleanest PR of the five. One request for the cluster generally: several of these PRs re-wrap comments in code they do not otherwise change, including in unrelated tests. It inflates review and creates conflicts between your own sibling PRs — #1604, #1607 and #1608 all rewrite the same snprintf block and all rename build_grep_cmd, so whichever lands second collides. Worth a formatter setting matched to the repo's column width. Thank you for #1565 and for putting five concrete PRs behind it. The pathological-scan problem is real and it is now on the roadmap with a shape we can support long-term. |
Sorry, something went wrong.
|
Closing in favor of the maintainer's planned cross-platform search_code pagination. Agreed that a producer-side row cap can silently underfill path-filtered results and report an incomplete count as complete. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
What does this PR do?
Caps every Windows PowerShell search_code producer at 500 results using Select-Object -First 500.
The cap is applied after producer-side filters and uses the same limit as the existing C-side acceptance cap. It does not introduce the previously proposed separate 4096-result limit.
Adds regression coverage for all four generated Windows PowerShell command paths and verifies the cap’s value and placement.
Test results: 7352 passed, 0 failed, 63 skipped.
Part of #1565
Checklist
unsigned commits (DCO, see CONTRIBUTING.md)