fix(vector): reject non-positive similar_to neighbor count

[eharris128]

Description

A similar_to() vector query takes a client-supplied number of neighbors. That argument is parsed in worker/task.go without a lower-bound check, so a zero or negative value flows into the HNSW search. On the default search path (no ef / distance_threshold option), SearchWithPath passes it through unclamped, and addPathNode (tok/hnsw/search_layer.go) computes effectiveMaxLen = maxResults + filtered and then evaluates slr.neighbors[:effectiveMaxLen]. A negative count panics with a negative slice bound ([:-1]); a count of zero empties the slice and then dereferences slr.neighbors[0]. Either way a single query with a neighbor count <= 0 panics the query goroutine and crashes the Alpha. The SearchWithOptions path already guards against this with an if maxResults < 0 { maxResults = 0 } clamp; only the default path was left unprotected.

This change:

• Rejects a non-positive neighbor count at the request boundary (worker/task.go) with a clear error, so an invalid similar_to count returns a normal query error instead of crashing the server.
• Mirrors the existing SearchWithOptions clamp into SearchWithPath as defense in depth.
• Clamps effectiveMaxLen and guards the slr.neighbors[0] dereference in addPathNode, so the bottom-layer search cannot panic on a non-positive limit regardless of caller.
• Adds a regression test (TestAddPathNodeNonPositiveMaxResults) that reproduces the panic on the old code and passes with the fix.

Valid queries are unaffected: a normal positive neighbor count truncates exactly as before.

Checklist

• The PR title follows the Conventional Commits syntax, leading with fix:, feat:, chore:, ci:, etc.
• Code compiles correctly and linting (via trunk) passes locally
• Tests added for new functionality, or regression tests for bug fixes added as applicable

[shiva-istari]

Thanks @eharris128 for the fix. The worker/task.go guard looks right and the root-cause write-up is solid. Two asks on the test side:

1. Can you add an integration test? The new unit test covers addPathNode in isolation, but the actual user-facing contract a bad k returns a query error and the Alpha stays up only lives in the worker/task.go guard, and nothing exercises it end-to-end. Both k=0 and k=-1 parse fine through DQL, so this slots right into query/vector/vector_test.go next to TestSimilarToOptionsIntegration: fire similar_to(pred, 0, ...) and similar_to(pred, -1, ...), assert both return an error containing "number of neighbors", then run a valid similar_to afterwards and assert it succeeds. That last query is the important bit on the old code the first bad query kills the server, so it's what makes this a real crash-regression test.

2. Tighten the new regression test a bit. TestAddPathNodeNonPositiveMaxResults only asserts "no panic", but the search loop also depends on the state addPathNode leaves behind worth asserting slr.neighbors (and slr.path) look sane after the call too, so the test pins the behavior and not just the absence of a crash.

[eharris128]

Thanks @shiva-istari — both good calls, addressed in 80227ff4e.

1. Integration test. Added TestSimilarToNonPositiveNeighbors in query/vector/vector_test.go, right next to TestSimilarToOptionsIntegration. It fires similar_to(pred, 0, ...) and similar_to(pred, -1, ...), asserts both return an error containing "number of neighbors", and then runs a valid similar_to(pred, 3, ...) and asserts it returns the expected UIDs. As you noted, that last query is the real point: on the old code the first non-positive query panics and takes down the Alpha, so a subsequent successful query is what proves this is a genuine crash regression and not just an error-message check.

2. Tightened the unit test. TestAddPathNodeNonPositiveMaxResults now pins the state addPathNode leaves behind, not just the absence of a panic. With a non-positive limit the clamp forces effectiveMaxLen to 0, so I assert neighbors truncates to empty and path is left exactly as setFirstPathNode set it. Asserting neighbors is empty is the honest post-clamp contract here — the degrade-to-empty is intentional.

No production code changed from the last review — both are test-only.

[eharris128]

(also my commit on the fork is still syncing it would appear) - in case you see my above comment before the commit renders here in the PR diff.