diff --git a/internal/mcp/explore_divergent_default_owner.go b/internal/mcp/explore_divergent_default_owner.go index d2b020c4..6140a5cc 100644 --- a/internal/mcp/explore_divergent_default_owner.go +++ b/internal/mcp/explore_divergent_default_owner.go @@ -20,6 +20,19 @@ const ( exploreDefaultOwnerFileCap = 3 exploreDefaultOwnerFileNodeCap = 64 exploreDefaultOwnerTotalCap = 96 + // exploreDefaultOwnerFallbackSLO bounds the ranked-callable fallback that + // runs when no base constructor/type pair was already ranked. It is a + // SILENT bound: every check inside + // exploreDivergentDefaultBasesFromRankedCallables returns "no bases" on + // expiry, so the caller simply promotes nothing and the response is a + // smaller symbol set with no error and no warning. + // + // 25ms is ample for the batched inbound lookup + node hydration on an idle + // machine and is routinely exceeded on a loaded one, where those SQLite + // reads are descheduled. That makes WHICH owner gets promoted a function + // of machine load — fine for production (the fallback is best-effort by + // design) and fatal for a test that asserts the promoted identity, which + // is why promoteExploreDivergentDefaultOwner takes an overridable slice. exploreDefaultOwnerFallbackSLO = 25 * time.Millisecond ) @@ -62,7 +75,13 @@ type exploreDivergentDefaultProjection struct { // lookup and one batched node hydration, and fails closed at every cap. A unique // match replaces its represented base pair before being moved to the head, so // promotion never evicts an unrelated protected candidate. -func promoteExploreDivergentDefaultOwner(task string, targets []exploreTarget, store graph.Store, maxSymbols int, readSource func(*graph.Node) string) []exploreTarget { +// The optional fallbackSLO overrides the wall-clock slice the ranked-callable +// fallback may spend. Omitted (or zero) means the production default. Only a +// caller that ASSERTS which owner gets promoted passes one, so its answer +// cannot depend on machine load — see the note on +// exploreDefaultOwnerFallbackSLO. Left variadic so the fifteen call sites that +// never reach the fallback stay untouched. +func promoteExploreDivergentDefaultOwner(task string, targets []exploreTarget, store graph.Store, maxSymbols int, readSource func(*graph.Node) string, fallbackSLO ...time.Duration) []exploreTarget { if store == nil || readSource == nil || len(targets) == 0 || !exploreQueryIsConceptTask(task) { return targets } @@ -72,7 +91,11 @@ func promoteExploreDivergentDefaultOwner(task string, targets []exploreTarget, s return targets } if len(bases) == 0 { - deadline := time.Now().Add(exploreDefaultOwnerFallbackSLO) + slo := exploreDefaultOwnerFallbackSLO + if len(fallbackSLO) > 0 && fallbackSLO[0] > 0 { + slo = fallbackSLO[0] + } + deadline := time.Now().Add(slo) bases, ok = exploreDivergentDefaultBasesFromRankedCallables(taskTerms, targets, store, deadline) if !ok || len(bases) == 0 { return targets diff --git a/internal/mcp/explore_divergent_default_owner_test.go b/internal/mcp/explore_divergent_default_owner_test.go index dc6c9da7..ba06f023 100644 --- a/internal/mcp/explore_divergent_default_owner_test.go +++ b/internal/mcp/explore_divergent_default_owner_test.go @@ -408,11 +408,16 @@ func TestHandleExplorePromotesDivergentDefaultOwnerFromSQLitePHPIndex(t *testing task := `StreamHandler::write reports "could not be opened: Permission denied" after a rotating handler applies chmod; find the divergent filePermission default and owning type` direct := promoteExploreDivergentDefaultOwner(task, []exploreTarget{{node: write}, {node: baseType}, {node: baseCtor}}, store, 3, func(node *graph.Node) string { return server.manifestSymbolSource(context.Background(), node) - }) + }, pinnedDivergentDefaultFallbackSLO) require.Equal(t, childCtor.ID, direct[0].node.ID, "real store projection did not promote child constructor") + // The fallback path is the one bounded by exploreDefaultOwnerFallbackSLO: + // with only `write` ranked there is no base constructor/type pair, so the + // ranked-callable projection runs and its batched SQLite reads must finish + // inside the slice. This assertion is about WHICH owner is promoted, not + // about how fast the machine is, so the slice is pinned. fallback := promoteExploreDivergentDefaultOwner(task, []exploreTarget{{node: write}}, store, 3, func(node *graph.Node) string { return server.manifestSymbolSource(context.Background(), node) - }) + }, pinnedDivergentDefaultFallbackSLO) require.Equal(t, childCtor.ID, fallback[0].node.ID, "real store callable-owner fallback did not promote child constructor") req := mcpgo.CallToolRequest{} @@ -515,7 +520,21 @@ class RotatingFileHandler extends StreamHandler { _, err = idx.Index(root) require.NoError(t, err) server := NewServer(query.NewEngine(store), store, idx, nil, zap.NewNop(), nil) - return server, store + return pinExploreDivergentDefaultFallbackSLO(server), store +} + +// pinnedDivergentDefaultFallbackSLO is the wall-clock slice tests give the +// divergent-default-owner fallback. Generous on purpose: these tests assert +// WHICH owner is promoted, and the production 25ms default makes that a +// function of machine load rather than of the ranking under test. +const pinnedDivergentDefaultFallbackSLO = 30 * time.Second + +// pinExploreDivergentDefaultFallbackSLO widens the fallback slice for a server +// whose explore responses are asserted symbol-by-symbol. Mirrors +// pinExploreSourceLiteralRecallBudget, for the same reason. +func pinExploreDivergentDefaultFallbackSLO(s *Server) *Server { + s.divergentDefaultFallbackSLOOverride = pinnedDivergentDefaultFallbackSLO + return s } func requirePHPFixtureNode(t *testing.T, store graph.Store, fileSuffix, name string, kind graph.NodeKind) *graph.Node { diff --git a/internal/mcp/server.go b/internal/mcp/server.go index 790dd194..5897119a 100644 --- a/internal/mcp/server.go +++ b/internal/mcp/server.go @@ -465,6 +465,15 @@ type Server struct { // pinExploreSourceLiteralRecallBudget. sourceLiteralRecallBudgetOverride time.Duration + // divergentDefaultFallbackSLOOverride replaces the wall-clock slice + // explore's divergent-default-owner fallback may spend. Test-only, same + // contract as sourceLiteralRecallBudgetOverride above: production leaves + // it zero and gets exploreDefaultOwnerFallbackSLO. A test that asserts + // WHICH owner the fallback promotes widens the slice so the answer cannot + // depend on how loaded the machine is. See + // pinExploreDivergentDefaultFallbackSLO. + divergentDefaultFallbackSLOOverride time.Duration + // critiqueLLMGenOverride substitutes the critique_review tool's LLM seam. // Test-only: production leaves it nil and the handler builds the closure // over llmService.Generate. A non-nil override stands in for the real diff --git a/internal/mcp/tools_explore.go b/internal/mcp/tools_explore.go index f98651ed..f5ca0f2b 100644 --- a/internal/mcp/tools_explore.go +++ b/internal/mcp/tools_explore.go @@ -2571,7 +2571,7 @@ func (s *Server) handleExplore(ctx context.Context, req mcp.CallToolRequest) (*m if exploreQueryIsConceptTask(task) && len(targets) > len(artifactTargets) { symbolTargets := promoteExploreDivergentDefaultOwner(task, targets[len(artifactTargets):], s.graph, maxSymbols, func(node *graph.Node) string { return s.manifestSymbolSource(ctx, node) - }) + }, s.divergentDefaultFallbackSLOOverride) targets = append(targets[:len(artifactTargets):len(artifactTargets)], symbolTargets...) } // Direct retrieval owns the ranked head. Once graph promotion has selected