fix(search_packages): relax nix package search constraints and score on the tool side - #647
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
Scott McMaster (scottmcmaster)
changed the title
![prelint[bot]](https://avatars.githubusercontent.com/in/2822344?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
The process_results sort closure calls relevance_score(a, &a.name) — scoring each result against its own name rather than the user's search query.
apps/native/src-tauri/src/evolve/search_packages.rs:265
Caution
When use_regex = true, the new code calls regex::escape(query) before passing it to nix search, which escapes all regex metacharacters and turns the query into a literal string search.
apps/native/src-tauri/src/evolve/search_packages.rs:84
2 finding(s) posted as inline comments.
|
| Option | What it gives users | What it costs | Effort to change later |
|---|---|---|---|
| Current (pool all, rank by name) | Best name match wins regardless of channel | Pinned-channel guarantee lost | Low: add a channel-weight input to relevance_score |
| Restore channel-order priority | Pinned channel wins for unambiguous queries | Later channels never contribute when quota fills | Medium: restructure collect loop |
| Explicit channel-preference weight | Users set a preference; scoring balances name and channel | New user-facing concept | High: requires UI and config schema |
3. Each channel's nix search results have no explicit candidate cap: the full output of every channel search flows into the scorer.
A query such as "python" or "vim" can return hundreds of results from nixpkgs. Three channels at hundreds of results each produces a large candidate pool. The scoring pass is fast, but the wall-clock cost of waiting for every channel's nix search process to complete before the agent sees any result is unbounded. The prior version of this PR included a per-channel cap for this reason. The current diff removes that cap with no replacement. Until PR-10 ships a per-tool deadline, a common-term search in a multi-channel setup can stall the evolution loop with no escape hatch.
| Option | What it gives users | What it costs | Effort to change later |
|---|---|---|---|
| Current (no cap) | Scorer sees all possible candidates | Unbounded latency on common queries | Low: add one cap constant |
| Per-channel cap (e.g. 100) | Bounded latency; scorer still gets a rich pool | Rare deep-channel match may be cut before scoring | Low: one constant |
| nix search --limit flag | nix itself truncates early | Behavior may differ across nix versions | Low to medium |
4. Platform-unavailable candidates are filtered after the agent's result count limit is applied, not before.
The new pipeline ranks candidates, truncates to the requested limit, then removes unavailable packages. If the top N results include M unavailable packages, the agent receives N minus M results rather than N. The old design continued collecting until it filled the quota with available packages. For most queries the practical impact is small because unavailable packages are rare. On an aarch64-darwin host querying a channel that carries many x86-only packages, the agent could receive far fewer results than requested, with no feedback explaining why the result set is small.
5. Derivation classification runs only on the final ranked set rather than on every candidate.
Classification reads derivation attributes to decide install method (cask vs. system package). Running it on every candidate before ranking wastes work on packages that will never reach the agent. The new design ranks cheaply first and classifies only the survivors. The test that counts classifier invocations confirms the behavior and guards against regressions.
6. Special characters in non-regex package names are now escaped for the underlying search tool, allowing literal searches like "c++".
The previous approach passed user terms directly as regex arguments, so a query like "c++" would be interpreted as a regex quantifier and produce no useful results. The new escaping converts each token to a literal POSIX ERE token before passing it to nix search. This is a correctness fix with no user-visible downside, and the new test covers the edge case.
Since the last review
- Still open: Scoring weight values were chosen by inspection and have not been validated against real eval cases showing search churn. (No eval run is referenced in the PR description or the single Graphite stack comment. Weight constants are unchanged from the prior diff.)
- Still open: Channel pooling removes the guarantee that a pinned or corporate channel's result surfaces first for unambiguous queries. (No channel-preference signal was added to relevance_score. The PR description and conversation contain no acknowledgment of this tradeoff.)
- Still open: Per-channel candidate cap bounds latency; removing the early-exit means all channels run to their cap before any results reach the agent. (No per-channel cap constant appears in the current diff. search_single_channel passes all nix search JSON output to process_search_output without a limit, and no --limit flag is present in the nix command construction.)
- Still open: Score weights were not validated against the eval cases that showed search churn, and no eval suite run is mentioned. (No eval results or eval run command appears in the PR description or comments.)
- Still open: No user segment using a corporate or pinned channel was identified, and the pooling change removes the channel-order guarantee with no acknowledgment. (No statement about pinned-channel users appears in the PR description or the single reviewer comment.)
- Still open: The per-tool deadline from PR-10 is not yet shipped, leaving multi-channel searches on common terms with no wall-clock bound. (The combined plan still lists PR-10 as a Phase 1 item. No deadline mechanism appears in this diff.)
Open questions
-
What does a common-term nix search (for example, "python" or "vim") return on a typical user nixpkgs channel, in terms of result count and wall-clock time? This determines whether the missing per-channel cap is an immediate latency problem or a theoretical one.
-
Has the scoring function been run against the eval cases that showed search churn, such as the Spotify case in the PR screenshots? Any result where the correct package ranks outside the top five would indicate a weight calibration problem.
-
Will PR-10's per-tool wall-clock deadline land immediately after this PR? Without it, three-channel common-term searches have no time bound.
Recommendation
Ship with changes
The anchor removal solves a real, screenshot-documented user problem and the scoring approach is the right product direction. The blocking issue is the missing per-channel candidate cap: without it, and without PR-10's per-tool deadline, nix search output on common terms is unbounded and the evolution loop has no escape hatch. Restoring a cap (the prior version of this PR had one) is a small change that directly addresses the one disagree verdict before shipping.
f4b793c to
4820a04
Compare
🎨 Storybook previewUpdated for f323f3a
|
📋 PR Overview
🔬 Coverage
|
4820a04 to
ea35db8
Compare
ea35db8 to
b4b7d9e
Compare
… likely" packages when scoring
b4b7d9e to
f323f3a
Compare
2 participants
Summary
This is for issue 617 -- another try at making the
search_packagestool return good results with less churn (my white whale).The ^ and $ anchors overconstrain the nix search so you can see this kind of churn:
With this change:
Then you can see much happier results more like this (note the scoring implied in the agent's evolution logs in the UI):
In addition to not missing pretty obvious things, by return just MORE results, we give the agent the opportunity to make better decisions in future search and edit steps.
Test Plan
Some new unit tests, plus manual testing.
Docs