Thank you for the contribution and for covering the semantic-only JSON path with end-to-end regression tests. This is now triaged as a high-priority MCP output correctness bug for 0.9.1-rc. Current main already contains an overlapping fix and regression coverage for #1295, so review will determine whether this branch still contains any distinct work. Our community PR queue is currently quite full, so it may take a little time before we can complete that review. We are doing our best to support community contributions and will return with code-grounded feedback as capacity opens.

Reviewed in full. The fix is sound, the three red checks are not your fault, and you were right that #1295 was not fully fixed. Sorry this sat — and sorry that our issue close implied the work was already done.

The catch is a good one. 0e24f84 fixed semantic-only isolation for the default tree output but never touched the legacy_json branch, so on today's main the format:"json" path still runs cbm_store_search unconditionally at src/mcp/mcp.c:3555. With no structural filters that matches the whole graph — which is exactly the total: 21143 dump in #1295, first entries coming from .github/issue-labeler.yml. Your shared has_structural_filters hoist keeps the two paths from drifting apart again, which is the right shape rather than patching one branch.

Good instinct running run_semantic_query_core before the document is built, too: the type-error path now returns before any allocation instead of discarding a completed structural search, and since the core sets *out_vresults=NULL / *out_vcount=0 up front and only transfers on success, nothing leaks on that path. I also checked the skipped-search case — out = {0} is safe for cbm_store_search_free (count-0 loop, free(NULL)), and the emitter produces total:0 / count:0 / groups:[].

The JSON half of your regression test is properly binding: on current main it fails with total:1 and unrelated_node present. Pinning the already-shipped default-format contract alongside it is welcome rather than redundant.

On the three failing checks — they are ours, not yours.

Your run tested a merge against main as of 28 July 17:34, which was mid-way through the Windows single-binary transition. Both failures come from that window and were fixed on main the same evening:

• pr-smoke (windows-latest) failed at FAIL: invalid Windows PATH smoke seam fell back to the live registry (scripts/smoke-test.sh:923). Fixed by 2740208, whose commit message describes that exact guard — it restores a non-zero exit status for a failed install --dry-run plan check.
• test / test-windows-guards failed on section_cold_storm / "cold-storm client 3 failed (racing daemon spawn)". Fixed by 5f2c146 (pinning Windows daemon startup interleaving instead of polling, plus a use-after-free it exposed) and 0a22fb1.
• ci-ok is just the aggregator of those two.

That guard has been green on several fully-green runs against current main since. And a diff confined to handle_search_graph plus a unit test has no causal path to daemon spawn or CLI install code.

What we need from you: rebase onto current main (or git merge origin/main) and push. Your branch is roughly 75 commits behind. One thing worth knowing — a plain "Re-run failed jobs" will not help here: it re-tests the same stale merge commit and will fail identically. The push is what refreshes the merge ref.

Once that run is green this is a merge candidate.

One cosmetic note, no action needed: your DCO sign-off name reads JJordan0K while your login is JJordan0C. The noreply email matches your account and the dco gate passed, so it is fine — just flagging it in case it was a typo you would rather fix.

Thank you for pushing on this after the issue was closed. You were right to.

Correction to my earlier comment: I cited the wrong commit SHA.

I said the default-path semantic-only isolation landed in 0e24f84. That is wrong. 0e24f84 is not on main at all — it lives on a local review branch and is itself an earlier rebase of PR #938, which carries the same title. Citing it as a merged fix was my error.

The isolation that is on main came from 4843a34 (the TOON compact-output work), and it is live today at src/mcp/mcp.c:3540 — bool semantic_only = sq_present && !has_filters; with the guard just below it.

Nothing about the substance of my review changes. The default (tree) path is isolated on main; the format:"json" path still runs cbm_store_search unconditionally, so the #1295 residual is real and this PR is the fix for it. The rebase request and the warning about "Re-run failed jobs" re-testing the same stale merge commit both still stand.

Apologies for the noise — a wrong SHA in a review is the kind of thing that wastes someone's time when they go looking for it.

Hi @DeusData, the branch has been updated with the latest main, and the refreshed CI is now fully green. Thanks again for the detailed review — this should be ready for another look.

Thank you for rebasing this and getting the checks green. The remaining semantic-only JSON search case is distinct from the older broad-search report, so I removed the incorrect duplicate label and kept this at high priority in 0.9.1-rc. It is back in the maintainer review queue, which is currently full, so the final re-review may take a little time.

Merged (de4ec9b) — thank you! The JSON path silently running the full structural search on semantic-only queries was the kind of bug only a careful reader of both format paths finds; the both-formats regression test seals it. #1295 closed.