[mache-cb8fb9] fix(graph): resolve callees from the container edge, not from a "source" child that leyline never writes - #616
Merged
Conversation
…ot from a "source" child that leyline never writes find_callees returned nothing for EVERY construct on a leyline projection — mache's primary backend. Reproduced before fixing: a two-function corpus where Caller calls Helper twice gave GetCallees(Caller) -> 0. The reported root cause was the missing `props` column, which does disable the scoped-extractor path. But that is not where it ends: the fallback then looks for a child node named "source" before consulting node_refs, and a leyline construct's children are parse-tree nodes — block, identifier, parameter_list, type_identifier. There is no "source" leaf, so sourceID came back empty and GetCallees returned nil BEFORE reaching the node_refs fallback that would have answered. The "source" child is a mache-schema shape being assumed on a schema that does not have it. The edge was already there. leyline writes node_refs rows whose container_node_id is the enclosing definition's node_id: token=Helper container_node_id=a.go/function_declaration_1 (= Caller) So a construct's callees are exactly the tokens its container owns. This derives nothing the producer had not already computed at parse time — it reads an edge mache was ignoring. Tried BEFORE the source-child lookup, since on a leyline db that lookup can only fail. DISTINCT is load-bearing: Helper is called twice, which is two call SITES and one callee. GetCallees answers "what does this call", not "how many times". Verified end-to-end on a real leyline projection: GetCallees(Caller) -> 1 [Helper] (was 0) GetCallees(Helper) -> 0 (correct — calls nothing) The regression lives in examples/publicapi, the consumer-shaped ladder, and asserts BOTH directions agree: what Run calls must name Run among its callers. Empty was the dangerous failure mode here — an agent cannot tell "this function calls nothing" from "this backend cannot answer", so it draws a wrong conclusion instead of asking a different question. Falsified rather than assumed: stubbing the container lookup fails the test. task check: exit 0, 42 packages.
…its root, instead of only in `mache init` The ?project= resolution path shipped, but nothing populated the thing it resolves against. registerProject had exactly one caller — `mache init` — so a daemon could serve a project for days with ~/.mache/projects.json absent entirely, and every token lookup necessarily missed. Found live rather than by reading: a supervised `mache serve --http` had been serving this repo for days with no registry file at all, no ?project= token in the client URL, and a client that could not answer roots/list. get_overview simply hung — the session never resolved a workspace, so there was no graph to query. Registration is now a byproduct of serving. Any session that learns a root — from client roots, from --path, from a stdio positional arg — records it, so the token exists for the NEXT client. That ordering is the whole point: the clients that most need token resolution (plain request/response HTTP, no channel for roots/list) are exactly the ones that can never populate the registry themselves. `mache init` keeps its distinct job — writing the ?project= token into the client's config, which a server cannot do for it. Deliberately non-fatal and quiet: a read-only HOME or corrupt registry must not take down a session that is otherwise resolving fine. Serving the graph is the job; registration is an optimization for later. The existence check keeps the steady state read-only — this runs per tool call on an unmapped session, and rewriting the file every time would be constant churn for no gain. The failure message now leads with `mache init` too. It previously said "configure MCP roots or start mache with an explicit --path" — and for the clients that actually reach that branch, roots is the one remedy they cannot act on. Naming it first sent people to fix the thing they could not fix. task check: exit 0, 42 packages.
…sion resolution was losing 98% of them
A cold review of this PR found the feature silently defeating its own
purpose, and reproduced it. Verified independently before fixing: 50
goroutines registering 50 distinct roots left 1-3 in the file.
registered 1/50 roots
registered 2/50 roots
registered 1/50 roots
The registry is a read-modify-write over one JSON file, and this now
runs on session resolution — which a shared HTTP daemon does
concurrently, one goroutine per request. Every caller wrote back a map
it had read BEFORE the others' inserts, so all but the last were
dropped. Nothing crashed and nothing was corrupted: writeFileAtomic
renames, so the file is never torn. It just lost almost everything,
which is worse — the token a later ?project= lookup needs was never
written, and the very burst this feature targets (many clients against a
shared daemon) is the case that loses the most.
Two locks, because there are two independent writers:
- a package mutex, for goroutines inside one daemon;
- an advisory flock, because `mache init`, a supervised
`mache serve --http`, and any number of `mache serve --stdio`
subprocesses are separate PROCESSES sharing ~/.mache/projects.json,
and a mutex cannot see across them.
Neither suffices alone. The lock lives on a dedicated .lock file rather
than on the registry itself: writeFileAtomic replaces the registry by
rename, so a lock held on it would end up on an unlinked inode while a
new one took its place, protecting nothing.
ensureProjectRegistered's check-then-write is now ONE critical section.
Split, two callers both observe "absent" and both proceed. It inlines
the write rather than delegating to registerProject because
withRegistryLock is not reentrant — flock on a second descriptor from
the same process deadlocks against itself on Linux.
Also from the same review:
- calleeTokensFromContainer swallowed ALL query errors as (nil, nil),
the exact thing its sibling calleeTokensFromRefs' doc comment warns
against. A transient DB fault would have surfaced to an agent as
"this function calls nothing". Now probes for the column with
ColumnExists — the convention cmd/smell_refs_views.go already
established for this same leyline-vs-native divergence — and
surfaces real failures.
- The unwritable-registry test asserted only NotPanics and discarded
the return value, so it would have passed had the write silently
SUCCEEDED. It now pins both halves of the contract: returns false,
and writes nothing.
The concurrency regression asserts an EXACT count. A lost-update bug
passes any weaker assertion, which is why there was no coverage before.
task check: exit 0, 42 packages.
jamestexas
force-pushed
the
fix/callees-leyline
branch
from
August 7, 2026 17:46
f1f1a1a to
bbb967a
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
find_calleeswas empty for every construct on a leyline-backed.db: leyline's projections give a construct's children as parse-tree nodes (block,identifier,parameter_list), never asourceleaf, so the existing lookup'ssourceIDcame back empty andGetCalleesreturned nil before any fallback ran.calleeTokensFromContainer, tried before the source-child lookup whenuseNodesTableis set: reads callees straight fromnode_refs.container_node_id, the edge leyline already writes at parse time — nothing new is derived, just read from where the producer put it.DISTINCTon the token query is load-bearing: a function calling the same helper twice is one callee, two call sites.Test plan
examples/publicapi/browse_test.goextended to cover the container-edge pathtask ci(full local gate, matches CI) green