fix(daemon): symbol probe answers no evidence for a path no corpus claims by davalillo · Pull Request #903 · zzet/gortex · GitHub
Skip to content

fix(daemon): symbol probe answers no evidence for a path no corpus claims - #903

Open
davalillo wants to merge 4 commits into
zzet:mainfrom
davalillo:pr/probe-no-evidence
Open

davalillo wants to merge 4 commits into
zzet:mainfrom
davalillo:pr/probe-no-evidence

Conversation

@davalillo

Copy link
Copy Markdown
Contributor

Summary

Two related probe fixes:

  1. fix(daemon): the symbol probe answered with stale/adjacent evidence for a path that no corpus claims — it now answers "no evidence" when no indexed corpus covers the probed path.
  2. docs(daemon): documents the exact semantics of ProbeView for dedicated checkouts, so CLI consumers can distinguish "this path is served by a dedicated checkout" from the automatic-lane answers.

Independent of the memory-backend work; applies cleanly on main.

Testing

  • go test -race ./cmd/gortex/... ./internal/daemon/... green (1455 tests).

Exact=true attests corpus ownership, not working-copy freshness: a base
answer for a dedicated checkout reads the family primary's snapshot under
the shared repo prefix, not the checkout's own uncommitted content. The
#15 clamp narrows the cross-repo dimension only. Comments only, no
behavior change.

Closes #17.

(cherry picked from commit 417f2a6)
…aims

A path-scoped symbol probe whose path sits outside every tracked repo
left searchScope empty and answered from the union graph: a hook could
deny an identifier-shaped edit citing symbols from arbitrary repos the
deny's own redirect targets cannot answer about — the untracked variant
of the false-deny shape #15 clamped for tracked paths (issue #18).

The probe now answers with no hits when a path is given but no corpus
claims it; the base view still names what answered. Path-less requests
keep the legacy wire shape unchanged.

Also re-pins the untracked contract explicitly (the old test pinned the
union behavior) and strengthens the clamp test's unscoped assertion to
file paths, replacing a len()==2-in-disguise.

Closes #18.

(cherry picked from commit 7f3d4fd)
The windows-latest failure is TestBulkLoadPersistSpeed, a timing-ratio
test (bulk vs plain persist speedup) that flakes on slow CI runners;
the fix(daemon) probe change does not touch the bulk-load path.
@davalillo
davalillo marked this pull request as ready for review October 21, 2026 21:16

@zzet zzet left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. The behavior change is small and the new test fails without it, so the code is close. I'm requesting changes for the comments and commit messages, because several of them send the reader to the wrong place.

Comments and history are what the next person reads to learn why the probe behaves the way it does. A reference that resolves to an unrelated item, or a description that doesn't match the code, is worse than no reference: the reader trusts it and goes the wrong way. Please make every reference resolve to the right thing in this repo, and every claim match what the code does.

1. Issue numbers from the fork

#15, #17 and #18 are davalillo/gortex issues. In this repo they resolve to unrelated, closed PRs:

  • #15 is "ci(deps): bump the actions-minor-patch group with 2 updates"
  • #17 is "Load workspace config when indexing tracked repos"
  • #18 is "Expand strings nodes in graph"

They appear in:

  • cmd/gortex/daemon_controller.go:1685-1686
  • cmd/gortex/daemon_controller_view_test.go:467, :500, :503
  • internal/daemon/proto.go:673
  • the commit messages of 0f36a48 and 71db353, including Closes #18. and Closes #17. This repo merges with merge commits, so those lines land on main as written and GitHub cross-links them from #17 and #18. The (cherry picked from commit …) trailers name fork commits that don't exist here.

Please:

  • In code comments, state the rule itself. Where a reference helps, the upstream clamp is #880.
  • In commit messages, drop Closes #17 / Closes #18 (or write davalillo/gortex#18), and drop the cherry-pick trailers.
  • If you want to keep the trail, put the fork issue links in the PR description, fully qualified.

2. A dedicated checkout doesn't read the family primary's snapshot

The new text in internal/daemon/proto.go:640-642 and :669-676, and in cmd/gortex/daemon_controller_view.go:175-177 and :241-244, says an exact base answer for a dedicated checkout reads "the family primary's snapshot under the shared prefix", not the working copy's uncommitted content. That isn't how dedicated checkouts work here:

  • Register and the promotion path both give a linked worktree its own prefix, <base>@<admin>, via dedicatedPrefixFor (internal/indexer/worktree_prefix.go). They index that worktree's own tree and attach its own watcher (internal/indexer/checkout_lifecycle.go:456-485, internal/indexer/checkout_promote.go:200-230).
  • For a dedicated checkout, ExplainView returns that owned graph's prefix, with the reason "the checkout is dedicated, so its own corpus answers directly" (internal/indexer/checkout_overview.go:560-565).

So an exact base answer for a dedicated checkout comes from the checkout's own corpus, uncommitted edits included once the watcher has indexed them. The real caveat is index lag: edits that aren't indexed yet aren't in the answer. Please rewrite these comments to say that. As written, they would lead the next reader to fix a problem that doesn't exist.

3. The guard's comment is narrower than the guard

cmd/gortex/daemon_controller.go:1681-1688 says the path "belongs to no tracked corpus" and that FileCoverage "already reports the path untracked". But p.Repo == "" && p.Path != "" is also true when:

  • no view catalog is wired (the first early return in selectProbeView);
  • ExplainView fails (the checkout_inaccessible fallback, whose own comment says the binding is "not a precondition for answering");
  • a ready dedicated checkout has no dedicated-graph row yet, so RepoPrefix is empty.

In the last two cases FileCoverage reports the path as tracked: pathTracked returns true once view.root is set, and fileGraphKey fills in the missing prefix with ResolvePrefix(root). All three cases fail open (no hits, so no deny), so behavior is fine; the comment just claims something the code doesn't guarantee.

The simplest fix is to make the comment say what the guard actually catches. If you'd rather keep the comment's rule, decide "untracked" explicitly in selectProbeView (err == nil && !binding.Matched && binding.RepoPrefix == "") and guard on that. For a matched checkout with no graph row, resolve the prefix from its root the way fileGraphKey does.

4. The untracked answer is still labelled exact

TestProbeOfUntrackedPathAnswersNoEvidence dropped assert.True(t, found.View.Exact) (around cmd/gortex/daemon_controller_view_test.go:516), but the view didn't change. It is still {Kind: base, RepoPrefix: "", Exact: true}, and recordProbeAnswer counts it as an exact base answer.

The new Exact doc says Exact=true means "the answer came from the indexed corpus that owns the path's repo prefix", and nothing owns this path. Please keep the assertion, and say in the Exact doc what an exact base answer with an empty RepoPrefix means, so the label, the test and the doc agree. If you think it shouldn't be exact, change the label in this PR and pin that instead.

None of this changes behavior. Once the comments and commit messages are fixed, I'm happy to approve.

@zzet

zzet commented Oct 23, 2026

Copy link
Copy Markdown
Owner

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants