fix(mcp): return semantic-only graph results as primary

[dpersek]

What does this PR do?

Fixes #915.

search_graph now returns vector-ranked semantic hits in the primary results array for semantic-only semantic_query calls, instead of returning broad structural results that can make vector-less nodes appear to win the ranking.

Combined structural + semantic calls still expose semantic_results, and non-array semantic_query values still return the existing validation error. The vector search path now also reports a full semantic candidate count so total and has_more stay meaningful with limit/offset pagination.

Local full-suite caveat: make -f Makefile.cbm test is not green in this environment, but the failing graph-contract/probe slice reproduces on a clean upstream/main worktree, so it is not introduced by this patch. Focused MCP proof and lint-ci are green.

Checklist

• Every commit is signed off (git commit -s) — required, CI rejects
  unsigned commits (DCO, see CONTRIBUTING.md)
• Tests pass locally (make -f Makefile.cbm test)
• Lint passes (make -f Makefile.cbm lint-ci)
• New behavior is covered by a test (reproduce-first for bug fixes)

[DeusData]

Thanks for the semantic_query fix for #915. Triage: high-priority query correctness bug.

Review should verify semantic-only calls put vector-ranked hits in the primary results array, combined structural+semantic behavior stays compatible, and total/has_more report the semantic candidate set rather than the broad structural fallback.

[DeusData]

The shape of this is right: one semantic-query collection feeding either the primary results (semantic-only) or the legacy semantic_results sidecar (combined calls), correct total/has_more/offset pagination on the vector path, tool description updated, and the non-array validation error preserved. 19/19 green.

One change requested: the refactor quietly switched yyjson_mut_obj_add_strcpy → yyjson_mut_obj_add_str in emit_vector_results. add_str borrows the pointer, so correctness now depends on yy_doc_to_str() always running before semantic_query_output_free(&sem) — which both call sites currently honor, but nothing enforces: any future reorder (or an early-return path added between emit and serialize) becomes a use-after-free in a public MCP handler. Please either restore add_strcpy (the copy cost is trivial at ≤ limit results and buys memory-safety by construction), or if you keep the borrow, add a loud comment at emit_vector_results + both call sites pinning the serialize-before-free invariant. strcpy is my preference.

With that addressed this is merge-ready.

[dpersek]

Implemented your preference and restored yyjson_mut_obj_add_strcpy for all four vector-result strings.

[DeusData]

Thanks @dpersek — reviewed in depth. The core fix is right and we want it: a semantic-only search_graph call (semantic query, no structural filters) should return the semantic hits as the primary results, not an unfiltered structural dump — that's the #915 bug, and aligning the JSON path with the TOON path is correct. The stable qsort+node_id pagination is a good call too.

Two changes before we can merge, both about not shipping a scale/contract foot-gun:

1. Bound the semantic-only fetch. Right now the paginated semantic-only path sets fetch_limit = candidate_total, so it re-scores the entire Function/Method/Class vector set and qsorts all of it on every call (unbounded by limit). On a large graph that's a heavy per-query cost on the hot path. Please keep a bounded page window (e.g. the limit*5 prefilter the combined path still uses, or a hard cap) rather than a full-corpus rescan.
2. Make total meaningful. total currently counts the whole corpus (no score threshold), so has_more is ~always true and a client paginates through the near-zero-cosine tail. Please make total/has_more reflect a thresholded match set (like bm25/regex total counts actual matches).

Also please rebase onto current main (the base has moved) and re-run so we can re-review at the new SHA. The output-shape change itself (results→vector, drop semantic_results sidecar for semantic-only, add search_mode/total/has_more) is accepted. Appreciate this — it's a real fix.

[DeusData]

Reviewed properly at last — apologies this took so long, because the delay is most of the problem here.

Your diagnosis of #915 was exactly right, and it is worth saying plainly: the unfiltered structural search being prepended, letting vector-less nodes win the ranking alphabetically, is precisely the bug. Main's default-path isolation follows the direction this PR established, and it is live today at src/mcp/mcp.c:3540. Your test coverage — pagination edges, INT_MAX offset, the combined-call sidecar staying untouched — is genuinely thorough.

What happened is that main moved out from under you, twice. Since your last rebase in mid-July, TOON was pruned in favour of the tree format, and then format:"json" was unified onto that same tree model. This PR's JSON half builds a bespoke legacy results object — a second JSON dialect that main deliberately deleted. And the #1295 residual it also addresses is now being fixed by #1319 within the current architecture, in about 30 lines. So this branch needs a ground-up rework rather than a rebase, and that is our review latency showing, not a deficiency in your work.

Two pieces here are still genuinely valuable and are not on main:

1. Honest pagination for semantic results — total, offset/limit, and has_more. Today semantic-only output has no offset support and no truncation signal at all, so hits beyond limit are silently invisible. That is itself a findability gap, and closing it is a win.
2. Deterministic ordering — your qsort with a node-id tie-break, replacing the O(n²) re-sort, gives page-stable results. Also worth having on its own.

Both would make a strong focused follow-up against current main.

Two things I would want split out and decided explicitly, because they cut results rather than reorder them:

• VS_MIN_MATCH_SCORE (0.10). PRIMARY searches would silently drop every candidate below that cosine score. For a weak-signal query, the sub-threshold hits are the answer — so a query that returns something today could return nothing. The threshold is not mentioned in the PR body prose, only in the schema text.
• CBM_VECTOR_SEARCH_CANDIDATE_CAP (250). Pagination can never reach past that window. The metadata stays honest about it, but deep hits become unreachable. Fair mitigation: main already fetches limit × 5, so at the default limit of 50 the window is the same 250 — it only bites above that.

Neither is wrong; both trade recall for precision, and that is a product call we want to make deliberately rather than inherit. Splitting them out lets each be judged on its own.

Also riding along undeclared: a store-API signature change (cbm_vector_search_policy_t plus a total_count out-param — permanent surface for what is presentation-layer paging), and has_filters semantics changes for include_connected and empty-string labels.

Suggested path: let #1319 take the #1295 JSON residual, and open a focused PR against current main with the pagination and deterministic ordering. If you would rather not carry that yourself, we can distill those two pieces with Co-Authored-By credit to you — your call, and I would rather ask than assume.

One correction I owe you for accuracy: I initially attributed main's isolation to a different commit while reviewing this. The correct one is 4843a34. Mentioning it because a wrong SHA sends people looking in the wrong place.