From 3d99a1181cb1b5767684f760af37303a5c093910 Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Mon, 10 Aug 2026 11:41:04 -0700 Subject: [PATCH 1/5] Auto-connect clear Copilot context matches --- .../neatcontext/src/copilot/mcp-bridge.mjs | 111 ++++++++++- tests/copilot-plugin.test.mjs | 178 ++++++++++++++++++ tests/routing-unconnected.test.mjs | 26 +-- 3 files changed, 292 insertions(+), 23 deletions(-) diff --git a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs index eb624f4..3248f12 100644 --- a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs +++ b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs @@ -34,6 +34,7 @@ import { switchPolicy } from "../core/routing.mjs"; import { assess, createRoutingIndex } from "../core/routing-candidates.mjs"; +import { tokenize } from "../core/routing-search.mjs"; import { applySelection, resolveContext } from "../core/selection.mjs"; const SERVER_INFO = { name: "neatcontext", version: "0.3.4" }; @@ -361,6 +362,83 @@ async function routingMenu(query) { : renderMenu(entries, options); } +function normalizeRoutingText(value) { + return typeof value === "string" ? value.trim().toLowerCase().replace(/\s+/g, " ") : ""; +} + +function isConfidentMatch(entry, query) { + const normalizedQuery = normalizeRoutingText(query); + const queryTokens = tokenize(query); + const exact = + normalizeRoutingText(entry.name) === normalizedQuery || + entry.aliases.some((alias) => { + const aliasTokens = tokenize(alias); + return ( + aliasTokens.length > 0 && + queryTokens.some((_, start) => + aliasTokens.every((token, offset) => queryTokens[start + offset] === token) + ) + ); + }); + return exact || entry.matched.length >= 2; +} + +// `get_context` is already the session asking the plugin to route this request. +// In auto mode, complete a clear first connection here rather than depending on +// the model to translate the returned shortlist into a second `use_context` +// call. Existing connections are never changed by this shortcut: leaving a +// context still needs the conversational follow-up judgment only the model has. +async function autoConnectClearMatch(query) { + if (typeof query !== "string" || query.trim().length === 0) { + return null; + } + const selection = await readSelection().catch(() => null); + if (selection?.available === false || selection?.contextId) { + return null; + } + + const [{ contexts }, state] = await Promise.all([listAllContexts(), readRouting()]); + if (resolveMode(state, sessionId()) !== "auto") { + return null; + } + + const ranked = await rankContexts(contexts, state, query, { connectedId: null }); + const decision = assess(ranked); + const leader = decision.verdict === "clear" ? ranked[0] : null; + const target = leader && contexts.find((context) => context.id === leader.id); + if (!target) { + return null; + } + + const entry = { + ...menuEntries([target], state)[0], + matched: leader.matched + }; + if (!isConfidentMatch(entry, query)) { + return null; + } + + const policy = switchPolicy(state, { + id: sessionId(), + targetId: target.id, + connectedId: null + }); + if (!policy.allowed) { + return null; + } + + await applySelection(target); + await noteDecision({ + sessionId: sessionId(), + from: null, + to: target.name, + mode: policy.mode, + reason: `clear query match: ${leader.matched.join(", ")}`, + requested: false + }); + return target.name; +} + // A shortlist needs three things: a request to match against, enough contexts // that narrowing gains anything, and at least one that actually matched. Any of // them missing and the full menu goes out instead — a session is never left @@ -529,6 +607,10 @@ async function handleMessage(message) { return; } + const autoConnected = + message.method === "tools/call" && message.params?.name === GET_CONTEXT_TOOL.name + ? await autoConnectClearMatch(message.params?.arguments?.query) + : null; const context = await activeContext(); if (dependsOnExtensions(message)) { await refreshExtensions(context); @@ -553,7 +635,7 @@ async function handleMessage(message) { } if (!isNotification && response) { - writeLine(await shapeResponse(message, response)); + writeLine(await shapeResponse(message, response, autoConnected)); } } @@ -595,7 +677,27 @@ async function withNotes(response, place, query) { }; } -async function shapeResponse(message, response) { +function prependAutoConnection(response, contextName) { + if (!contextName || !Array.isArray(response.result?.content) || response.result.content[0]?.type !== "text") { + return response; + } + const content = response.result.content; + return { + ...response, + result: { + ...response.result, + content: [ + { + ...content[0], + text: `Automatically connected "${contextName}" for this request.\n\n${content[0].text}` + }, + ...content.slice(1) + ] + } + }; +} + +async function shapeResponse(message, response, autoConnected = null) { if (message.method === "initialize" && response.result) { return withNotes(response, "instructions"); } @@ -605,7 +707,10 @@ async function shapeResponse(message, response) { if (message.method === "tools/call" && message.params?.name === GET_CONTEXT_TOOL.name) { // The handshake has no request to match against, so only this path can // narrow the menu — which is also the path that is re-read every turn. - return withNotes(response, "content", message.params?.arguments?.query); + return prependAutoConnection( + await withNotes(response, "content", message.params?.arguments?.query), + autoConnected + ); } return response; } diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index 543ad9a..105eb23 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -862,6 +862,7 @@ test("Copilot narrows the routing menu to the request", async (t) => { for (const [name, useWhen] of corpus) { await createContext(home, name, { sessionId: "copilot-shortlist", useWhen }); } + await runNode(cli, ["mode", "ask"], { env }); const session = rpcSession(env); sessions.push(session); @@ -891,3 +892,180 @@ test("Copilot narrows the routing menu to the request", async (t) => { assert.match(unmatched.result.content[0].text, /## Contexts available on this machine/); assert.match(unmatched.result.content[0].text, /Docker container/); }); + +test("Copilot get_context auto-connects a uniquely clear first context in auto mode", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "LM coordination", { + sessionId, + useWhen: "LM-PF coordination implemented in Windows ServiceManager" + }); + await createContext(home, "Queue lag", { + sessionId, + useWhen: "order-events partition lag and consumer rebalancing" + }); + const alias = await runNode( + cli, + ["alias", "LM coordination", "--called", "LM coordination implemented in Windows ServiceManager"], + { env } + ); + assert.equal(alias.code, 0); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const response = await session.call( + toolCall(2, "get_context", { + query: "How is LM coordination implemented in Windows ServiceManager?" + }) + ); + assert.match(response.result.content[0].text, /Automatically connected "LM coordination"/); + assert.match(response.result.content[0].text, /connected context: LM coordination/i); + assert.doesNotMatch(response.result.content[0].text, /No NeatContext Context is connected/); +}); + +test("Copilot get_context does not auto-connect a near-tie", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-tie-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect-tie"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Codex packaging", { + sessionId, + useWhen: "plugin packaging, manifests and marketplace steps" + }); + await createContext(home, "Kimi packaging", { + sessionId, + useWhen: "plugin packaging, manifests and marketplace steps" + }); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const response = await session.call( + toolCall(2, "get_context", { query: "plugin packaging manifests marketplace steps" }) + ); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + assert.match(response.result.content[0].text, /Codex packaging/); + assert.match(response.result.content[0].text, /Kimi packaging/); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); +}); + +test("Copilot get_context does not auto-connect a weak one-term match", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-weak-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect-weak"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Collections", { + sessionId, + useWhen: "guide" + }); + const alias = await runNode(cli, ["alias", "Collections", "--called", "id"], { env }); + assert.equal(alias.code, 0); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const response = await session.call(toolCall(2, "get_context", { query: "guide" })); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + assert.match(response.result.content[0].text, /Collections/); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); +}); + +test("Copilot get_context does not auto-connect a context declined this session", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-declined-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect-declined"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + const declined = await session.call( + toolCall(2, "use_context", { context: "Checkout incident", declined: true }) + ); + assert.equal(declined.result.isError, false); + + const response = await session.call( + toolCall(3, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); +}); + +test("Copilot get_context preserves ask mode for a clear match", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-ask-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect-ask"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + const mode = await runNode(cli, ["mode", "ask"], { env }); + assert.equal(mode.code, 0); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const response = await session.call( + toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + assert.match(response.result.content[0].text, /Routing is on \(ask\)/); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); +}); + +test("Copilot get_context never auto-switches an existing connection", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-switch-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-switch"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Current work", { + sessionId, + useWhen: "the current connected work" + }); + await createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + const use = await runNode(cli, ["use", "Current work"], { env }); + assert.match(use.stdout, /Connected the "Current work" context/); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const response = await session.call( + toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.match(response.result.content[0].text, /connected context: Current work/i); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); +}); diff --git a/tests/routing-unconnected.test.mjs b/tests/routing-unconnected.test.mjs index 4d57330..284b812 100644 --- a/tests/routing-unconnected.test.mjs +++ b/tests/routing-unconnected.test.mjs @@ -176,16 +176,9 @@ describe("get_context with nothing connected", () => { await session.send("initialize", { protocolVersion: "2025-11-25" }); const text = await ask(session, "why is checkout-api throwing 5xx?"); - assert.match(text, /Connect the one this request belongs to with `use_context`/); - assert.match(text, /do not ask the user to run a command/); - // The regression itself: the old text opened by telling the model to send - // the user to /neatcontext:use, and that is what it acted on. - assert.doesNotMatch( - text.split("## Contexts")[0], - /\/neatcontext:use/, - "the lead paragraph must not answer 'what now?' with a slash command" - ); - assert.match(text, /Incident/); + assert.match(text, /Automatically connected "Incident"/); + assert.match(text, /connected context: Incident/i); + assert.doesNotMatch(text, /No NeatContext Context is connected/); } finally { await session.close(); } @@ -199,16 +192,9 @@ describe("get_context with nothing connected", () => { const session = bridge("upgraded-machine"); try { await session.send("initialize", { protocolVersion: "2025-11-25" }); - assert.match(await ask(session, "checkout-api 5xx"), /Routing is on \(auto\)/); - - // And the switch it was told to make actually goes through, unprompted, - // which is what the baked-in ask was refusing. - const used = await session.send("tools/call", { - name: "use_context", - arguments: { context: "Incident", reason: "checkout 5xx" } - }); - assert.equal(used.result.isError, false); - assert.match(used.result.content[0].text, /Switched this session to "Incident"/); + const text = await ask(session, "checkout-api 5xx"); + assert.match(text, /Automatically connected "Incident"/); + assert.match(text, /connected context: Incident/i); } finally { await session.close(); } From 57ae7c122344489a3ca8634e8f7e4ba8de33d604 Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Mon, 10 Aug 2026 18:00:51 -0700 Subject: [PATCH 2/5] test(copilot): verify automatic context routing --- .../neatcontext/src/copilot/mcp-bridge.mjs | 31 ++++++---- tests/copilot-plugin.test.mjs | 60 ++++++++++++++++++- 2 files changed, 77 insertions(+), 14 deletions(-) diff --git a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs index 3248f12..99cad5a 100644 --- a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs +++ b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs @@ -45,7 +45,10 @@ const GET_CONTEXT_TOOL = { "Get the connected NeatContext Context: domain profile files to read, and local " + "knowledge folders to search. Call this before answering anything that depends on the " + "user's own domain, documents, tools, or team conventions — some hosts do not surface " + - "this server's initialize instructions, so the tool description is what carries that rule.", + "this server's initialize instructions, so the tool description is what carries that rule. " + + "Pass the user's request as query before calling use_context: when nothing is connected, " + + "this tool safely auto-connects a uniquely clear match in auto mode or returns the routing " + + "menu needed to choose, ask, or decline.", inputSchema: { type: "object", properties: { @@ -91,11 +94,11 @@ const NOTHING_CONNECTED = // contradicting it. const NOTHING_CONNECTED_ROUTABLE = `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + - "one is for. Connect the one this request belongs to with `use_context`, then call " + - "`get_context` again and answer from what it returns — do not ask the user to run a command " + - "to connect a context you can already name. If none of them covers the request, say so and " + - "offer `/neatcontext:save` to make one out of this conversation. Until then, do not answer " + - "from general knowledge."; + "one is for. No safe automatic match was made for this call. Follow the routing " + + "rules below: connect a clear choice with `use_context`, ask when the choice is ambiguous, " + + "or say none covers the request. Do not ask the user to run a command to connect a context " + + "you can already name. If none covers the request, offer `/neatcontext:save` to make one out " + + "of this conversation. Until then, do not answer from general knowledge."; const NOTHING_CONNECTED_ASK = `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + @@ -116,7 +119,7 @@ const CONNECTION_RULE = `## Connecting a context, in GitHub Copilot Contexts are connected from this session and nowhere else: the \`use_context\` tool, or \`/neatcontext:use \` run by the user. \`/neatcontext:disconnect\` disconnects the current one from this session. New ones are made from here too: \`/neatcontext:save\` turns the work in this conversation into one, and \`/neatcontext:create\` builds one around a folder of documents the user already has. -There is no Desktop connection right now. Contexts are stored by this plugin. When the connected context is the wrong one, or none is connected, name the one you need and connect it here with \`use_context\` — or offer to, when the routing rules above say to ask first.`; +There is no Desktop connection right now. Contexts are stored by this plugin. When a request may need a context, call \`get_context\` with the user's request before \`use_context\`. With nothing connected, it safely auto-connects a uniquely clear match in auto mode; otherwise it returns the current routing menu. Use \`use_context\` only to act on that menu, switch a wrong connection, or honor an explicit user choice — and ask first when the routing rules say to ask.`; // The two tools that let a session change what it is grounded in. They are the // plugin's whole routing mechanism: there is no model in any process here, so @@ -125,11 +128,12 @@ const USE_CONTEXT_TOOL = { name: "use_context", title: "Switch Context", description: - "Switch this session to a different NeatContext Context, then call get_context and " + - "answer from what it returns. Name the context exactly as the routing menu lists it. " + - "In ask mode this only succeeds once the user has agreed — set `requested` then. Set " + - "`declined` instead of switching when the user turns a suggested switch down, so it is " + - "not suggested again.", + "Act on a routing menu returned by get_context, switch a wrong connection, or honor an " + + "explicit user choice; do not call this before get_context when routing a new request. " + + "After switching, call get_context and answer from what it returns. Name the context " + + "exactly as the routing menu lists it. In ask mode this only succeeds once the user has " + + "agreed — set `requested` then. Set `declined` instead of switching when the user turns a " + + "suggested switch down, so it is not suggested again.", inputSchema: { type: "object", properties: { @@ -203,7 +207,8 @@ These instructions are fixed at the handshake and cannot be updated, so they are When the user asks anything that depends on their own domain, documents, tools, or team conventions, call the get_context tool and let its answer decide: - If it returns a Context, ground your answer in it and cite what you used. -- If it reports that nothing is connected, it also lists the contexts that exist and says what to do about them — which may be to connect one yourself with the use_context tool, to ask the user first, or to tell them to run a command. Do what that answer says. It knows the current state and this text does not, so never substitute a slash command of your own for the route it offers.`; +- Pass the user's request as query. If nothing is connected, get_context safely auto-connects a uniquely clear match in auto mode or returns the current routing menu. +- If it still reports that nothing is connected, follow that returned menu: use use_context only for its clear choice, ask the user when required, or say no context covers the request. It knows the current state and this text does not, so never substitute a slash command of your own for the route it offers.`; function writeLine(message) { process.stdout.write(`${JSON.stringify(message)}\n`); diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index 105eb23..f9270c6 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -692,6 +692,14 @@ test("Copilot MCP bridge serves Contexts and routing locally", async (t) => { const initialized = await session.call(initialize(1)); assert.equal(initialized.result.serverInfo.name, "neatcontext"); assert.match(initialized.result.instructions, /get_context/); + assert.match( + initialized.result.instructions, + /call `get_context` with the user's request before `use_context`/ + ); + assert.doesNotMatch( + initialized.result.instructions, + /none is connected, name the one you need and connect it here with `use_context`/ + ); assert.match(initialized.result.instructions, /Connecting a context, in GitHub Copilot/); assert.match(initialized.result.instructions, /no Desktop connection right now/); @@ -706,7 +714,8 @@ test("Copilot MCP bridge serves Contexts and routing locally", async (t) => { assert.match(empty.result.content[0].text, /No NeatContext Context is connected/); // A context exists and routing is on, so the answer leads with the route this // session can take itself rather than with a command for the user to type. - assert.match(empty.result.content[0].text, /Connect the one this request belongs to/); + assert.match(empty.result.content[0].text, /No safe automatic match was made for this call/); + assert.match(empty.result.content[0].text, /connect a clear choice with `use_context`/); // Ask mode refuses an unrequested switch. It is set explicitly rather than // assumed, so this keeps testing ask mode whatever the default becomes. @@ -928,6 +937,21 @@ test("Copilot get_context auto-connects a uniquely clear first context in auto m assert.match(response.result.content[0].text, /Automatically connected "LM coordination"/); assert.match(response.result.content[0].text, /connected context: LM coordination/i); assert.doesNotMatch(response.result.content[0].text, /No NeatContext Context is connected/); + + const routing = JSON.parse( + await readFile(path.join(home.directory, "plugin-routing.json"), "utf8") + ); + assert.deepEqual(routing.decisions, [ + { + at: routing.decisions[0].at, + sessionId, + from: null, + to: "LM coordination", + mode: "auto", + reason: "clear query match: lm, coordination, implemented, windows, servicemanager", + requested: false + } + ]); }); test("Copilot get_context does not auto-connect a near-tie", async (t) => { @@ -985,6 +1009,40 @@ test("Copilot get_context does not auto-connect a weak one-term match", async (t assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); }); +test("Copilot get_context leaves an unrelated request unconnected", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-unrelated-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect-unrelated"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "LM coordination", { + sessionId, + useWhen: "LM-PF coordination implemented in Windows ServiceManager" + }); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const response = await session.call( + toolCall(2, "get_context", { query: "What is the capital of France?" }) + ); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + assert.match(response.result.content[0].text, /LM coordination/); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); + + const routing = JSON.parse( + await readFile(path.join(home.directory, "plugin-routing.json"), "utf8") + ); + assert.deepEqual(routing.sessions, {}); + assert.deepEqual(routing.decisions, []); + await assert.rejects(stat(path.join(home.directory, "plugin-sessions")), { + code: "ENOENT" + }); +}); + test("Copilot get_context does not auto-connect a context declined this session", async (t) => { const home = await isolatedHome("neatcontext-copilot-auto-connect-declined-"); const sessions = []; From 3219921f43832593a8c8e1376f78fc1e82934d1b Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Mon, 10 Aug 2026 23:06:57 -0700 Subject: [PATCH 3/5] Gate auto-connect behind an absolute confidence floor Review found the clear-match path could connect on evidence far weaker than the menu it replaced: one common term, a filename hit, or a short alias found inside an unrelated sentence. Add an absolute floor beside `assess`, which only ever judged the leader relative to the field and so always said "clear" for a single-context store. A match now needs two agreeing terms the user actually typed, counted as words rather than as index tokens, and terms landing only in `files` do not count. An exact name, or an alias the user wrote, still stands in for the floor -- an alias of one word has to be the whole request, and a longer one has to survive tokenizing as two words and appear contiguously, so `the api` cannot route every sentence containing `api`. Also: - rank, boost, then slice, so decline and familiarity multipliers can promote a context into the visible set instead of being applied to a slice that was already cut - exclude the bridge's own automatic connections from familiarity, so routing cannot teach itself a preference the user never expressed - carry the near-tie note into the menu the bridge renders when it declines - make one pass over the store per call and thread it through, rather than re-reading the selection, the listing and the routing state up to three times for one answer - require a host-published session id, so a keyword hit in one window cannot re-ground a conversation in another - guard the whole ranking and persistence tail: an auto-connection that cannot be made is a missed optimization, and unguarded it left every `get_context` in the session unanswered --- .../src/core/routing-candidates.mjs | 130 +++++++- .../neatcontext/src/core/routing-search.mjs | 18 +- .../plugins/neatcontext/src/core/routing.mjs | 27 +- .../src/core/routing-candidates.mjs | 130 +++++++- .../neatcontext/src/core/routing-search.mjs | 18 +- .../neatcontext/src/core/routing.mjs | 27 +- .../neatcontext/src/copilot/mcp-bridge.mjs | 291 +++++++++++------- .../neatcontext/src/copilot/session.mjs | 16 + .../src/core/routing-candidates.mjs | 130 +++++++- .../neatcontext/src/core/routing-search.mjs | 18 +- .../copilot/neatcontext/src/core/routing.mjs | 27 +- .../src/core/routing-candidates.mjs | 130 +++++++- .../neatcontext/src/core/routing-search.mjs | 18 +- .../neatcontext/src/core/routing.mjs | 27 +- .../src/core/routing-candidates.mjs | 130 +++++++- .../neatcontext/src/core/routing-search.mjs | 18 +- plugins/pi/neatcontext/src/core/routing.mjs | 27 +- shared/core/routing-candidates.mjs | 130 +++++++- shared/core/routing-search.mjs | 18 +- shared/core/routing.mjs | 27 +- tests/copilot-plugin.test.mjs | 197 +++++++++++- tests/routing-confidence.test.mjs | 190 +++++++++++- tests/routing-unconnected.test.mjs | 64 ++++ 23 files changed, 1639 insertions(+), 169 deletions(-) diff --git a/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs b/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs index b452f57..ce0eeb8 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs @@ -19,7 +19,7 @@ // checking costs no extra reads. Only a real change pays for a rebuild. import { declineFactor, familiarity } from "./routing.mjs"; -import { buildIndex, rank } from "./routing-search.mjs"; +import { buildIndex, rank, tokenize } from "./routing-search.mjs"; // The fields, in the shape the scorer weighs. Aliases are joined into one // string rather than kept as a list because the scorer counts words, and a @@ -97,6 +97,118 @@ export function assess(ranked) { return { verdict: leaders.length > 1 ? "close" : "clear", leaders }; } +// --- acting on a match without being asked ----------------------------------- +// +// `assess` answers "is one candidate ahead of the others", which is a question +// about the shape of the ranking. It is not the same question as "is this good +// enough to re-ground a session on", and a store of one context makes the +// difference plain: the leader is always uncontested there, so `clear` comes +// back on any query that matched a single word. +// +// So the floor below is absolute rather than relative. It asks how much of the +// request actually agreed with this context, and it lives here — beside +// `assess`, in host-neutral core — because nothing about it is specific to one +// host. Every bridge reads and writes the same `~/.neatcontext`; a rule about +// when routing may act unasked cannot be one host's private opinion. + +// How many independent parts of the request have to agree. +const MIN_AGREEING_TERMS = 2; + +// Where a hit has to land to count toward that floor. `FIELD_WEIGHTS` already +// rates `files` lowest because a knowledge folder's listing is incidental to +// what a context is *for*; a floor that counted filenames equally would throw +// that distinction away at the one moment it matters most, and "where is the +// deploy runbook?" would connect on two filenames and nothing else. +const INCIDENTAL_FIELDS = new Set(["files"]); + +export function normalizeRoutingText(text) { + return text.trim().toLowerCase().replace(/\s+/g, " "); +} + +// The request split the way the user wrote it, deduplicated: one entry per +// whitespace-separated run. +// +// This is the unit the floor counts, and it has to be, because a token is not +// one. `tokenize` deliberately expands a single `checkout-api` into +// `[checkout-api, checkout, api]` and a two-character CJK request into seven +// tokens — good for recall, but counting those as agreement means one word, or +// any CJK request at all, clears a floor meant to require two. +function queryTerms(query) { + return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); +} + +function agreeingTerms(candidate, query) { + const carried = new Set( + (candidate.matched ?? []).filter((term) => { + const fields = candidate.matchedFields?.[term]; + return !fields || fields.some((field) => !INCIDENTAL_FIELDS.has(field)); + }) + ); + if (carried.size === 0) { + return 0; + } + let agreeing = 0; + for (const term of queryTerms(query)) { + if (tokenize(term).some((token) => carried.has(token))) { + agreeing += 1; + } + } + return agreeing; +} + +// An alias is the one routing signal the user authored by hand, at the moment +// they were correcting a wrong route, so it may stand in for the term floor. +// Only when it is specific enough to be evidence, though: a one-word alias +// found inside a longer sentence is weaker than the rule it would be skipping, +// and `api`, `pr` or `lm` are exactly the aliases people write. A one-word +// alias therefore has to be the whole request; a longer one has to appear +// contiguously in the request's tokens. +// +// One word means one word the user typed, counted the way `queryTerms` counts +// the request — not the tokens the index derived from it. `tokenize` expands +// `checkout-api` into three and `user_id` into three, and reading that as a +// multi-word alias would reopen the bypass for every ticket id, service name +// and API version anyone is likely to register. +// +// It has to survive tokenizing as two, as well. `the api` is two words the user +// typed, but `tokenize` drops the stopword and leaves one, and a one-token +// contiguous check is just "does this word appear anywhere" — the very test the +// first floor exists to prevent. `the API`, `our PR`, `how LM works` are how +// people write these aliases down, so both floors have to hold. +function wordCount(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean).length; +} + +function matchesAlias(aliases, query) { + const normalized = normalizeRoutingText(query); + const queryTokens = tokenize(query); + return aliases.some((alias) => { + const aliasTokens = tokenize(alias); + if (aliasTokens.length === 0) { + return false; + } + if (wordCount(alias) < 2 || aliasTokens.length < 2) { + return normalizeRoutingText(alias) === normalized; + } + return queryTokens.some((_, start) => + aliasTokens.every((token, offset) => queryTokens[start + offset] === token) + ); + }); +} + +// Whether a leading candidate is strong enough to connect to without asking. +// `assess` has to have said `clear` first — this only decides whether the +// leader earned it. +export function isConfidentMatch(candidate, query, { aliases = [] } = {}) { + if (typeof query !== "string" || query.trim().length === 0) { + return false; + } + if (normalizeRoutingText(candidate?.name ?? "") === normalizeRoutingText(query)) { + return true; + } + return matchesAlias(aliases, query) || agreeingTerms(candidate, query) >= MIN_AGREEING_TERMS; +} + export function createRoutingIndex({ listFiles }) { let key = null; let index = null; @@ -109,14 +221,20 @@ export function createRoutingIndex({ listFiles }) { } const byId = new Map(contexts.map((context) => [context.id, context])); const now = new Date(); - const { connectedId = null } = options; + const { connectedId = null, limit = 5 } = options; // Past refusals are applied after ranking rather than folded into the // index: they change on their own schedule, and rebuilding the index every // time someone says no would throw away the cache for a multiplier. // // Re-sorted afterwards because a discount can change the order, and the // shortlist's whole meaning is that it is in order. - return rank(index, query, options) + // + // And cut to `limit` only after that, never before. `rank` slices on raw + // BM25, so a candidate that wins once its decline and familiarity + // multipliers are applied could be dropped before anything here ever saw + // it — silently, and most damagingly for `assess`, which would then report + // an uncontested leader because its rival had been cut. + return rank(index, query, { ...options, limit: Number.POSITIVE_INFINITY }) .map((result) => { const context = byId.get(result.id); return { @@ -126,9 +244,11 @@ export function createRoutingIndex({ listFiles }) { result.score * declineFactor(state, result.id, now) * familiarity(state, context, { connectedId, now }), - matched: result.matched + matched: result.matched, + matchedFields: result.matchedFields }; }) - .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)); + .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) + .slice(0, limit); }; } diff --git a/codex-marketplace/plugins/neatcontext/src/core/routing-search.mjs b/codex-marketplace/plugins/neatcontext/src/core/routing-search.mjs index ff660e4..22c1767 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/routing-search.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/routing-search.mjs @@ -161,10 +161,18 @@ function addPosting(postings, token, id, field) { // Ranked candidates, best first, each with the query terms that put it there. // Those terms are the "why it matched" the session's model gets to read, and // they are the reason a caller can explain a route instead of asserting one. +// +// `matchedFields` says *where* each of those terms landed, which is the same +// distinction `FIELD_WEIGHTS` already makes and for the same reason: a hit in +// an alias the user wrote is evidence, and a hit in a filename picked up from a +// folder listing is a coincidence. Scoring weighs them apart; a caller deciding +// whether a match is strong enough to act on unasked needs to as well, and it +// cannot recover the field from the term alone. export function rank(index, query, { limit = 5 } = {}) { const terms = [...new Set(tokenize(query))]; const scores = new Map(); const matches = new Map(); + const landed = new Map(); for (const term of terms) { const byDocument = index.postings.get(term); @@ -183,11 +191,19 @@ export function rank(index, query, { limit = 5 } = {}) { } scores.set(id, (scores.get(id) ?? 0) + (idf * weighted) / (K1 + weighted)); matches.set(id, [...(matches.get(id) ?? []), term]); + const byTerm = landed.get(id) ?? new Map(); + byTerm.set(term, [...byField.keys()]); + landed.set(id, byTerm); } } return [...scores] - .map(([id, score]) => ({ id, score, matched: matches.get(id) })) + .map(([id, score]) => ({ + id, + score, + matched: matches.get(id), + matchedFields: Object.fromEntries(landed.get(id)) + })) .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) .slice(0, limit); } diff --git a/codex-marketplace/plugins/neatcontext/src/core/routing.mjs b/codex-marketplace/plugins/neatcontext/src/core/routing.mjs index 5c8da96..a0a401c 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/routing.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/routing.mjs @@ -383,11 +383,18 @@ export function declineFactor(state, contextId, now = new Date()) { // Names are unique in a store, so this is exact; renaming a context forgets its // history, which for a hint of this size is a fair trade against threading an // id through five hosts' bridges. +// +// Routes the plugin made for itself are skipped. They are in the log because +// the log is the record of what happened, but they are not evidence about this +// user: reading them back would raise the multiplier of a context this machine +// chose on a keyword hit, making the same choice likelier next time and the one +// after — a mis-route that argues for itself. Only what a person or a model +// decided counts as familiarity. export function familiarity(state, context, { connectedId = null, now = new Date() } = {}) { const sticky = context.id === connectedId ? STICKY_BOOST : 1; let weight = 0; for (const decision of state.decisions ?? []) { - if (decision?.to !== context.name) continue; + if (decision?.to !== context.name || decision?.automatic === true) continue; const days = (now.getTime() - Date.parse(decision.at)) / DAY_MS; if (!Number.isFinite(days) || days < 0) continue; weight += 0.5 ** (days / FRECENCY_HALF_LIFE_DAYS); @@ -414,6 +421,13 @@ function pruneDeclines(declines, now) { // Every switch, with what it was routing away from and why. Thresholds and card // quality are guesses until this has something in it: manual selections are the // ground truth that says whether the derived lines actually route correctly. +// +// Which is why a route the plugin made for itself has to be marked `automatic` +// as it goes in. Left indistinguishable, machine routes accumulate at roughly +// one per new session and the log stops being able to answer the question it is +// kept for. `requested` does not carry that distinction — a model calling +// `use_context` in auto mode records `requested: false` too, and it was still a +// decision somebody made. export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); @@ -451,7 +465,12 @@ function describe(entry) { // an instruction from a context you are *not* connected to is still an // instruction sitting in the window, and it would bleed into how the session // answers on the context you are. -export function renderMenu(entries, { connectedId, mode } = {}) { +// +// It takes a `decision` for the same reason the shortlist does. A near-tie is +// something the plugin knows and the model cannot see, and a store too small +// for a shortlist is exactly where the full menu goes out instead — so leaving +// the note behind there means the one caller that most needs it never gets it. +export function renderMenu(entries, { connectedId, mode, decision } = {}) { if (mode === "manual" || entries.length === 0) { return null; } @@ -461,6 +480,10 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); + const tie = tieNote(decision); + if (tie) { + lines.push(tie); + } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } diff --git a/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs b/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs index b452f57..ce0eeb8 100644 --- a/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs @@ -19,7 +19,7 @@ // checking costs no extra reads. Only a real change pays for a rebuild. import { declineFactor, familiarity } from "./routing.mjs"; -import { buildIndex, rank } from "./routing-search.mjs"; +import { buildIndex, rank, tokenize } from "./routing-search.mjs"; // The fields, in the shape the scorer weighs. Aliases are joined into one // string rather than kept as a list because the scorer counts words, and a @@ -97,6 +97,118 @@ export function assess(ranked) { return { verdict: leaders.length > 1 ? "close" : "clear", leaders }; } +// --- acting on a match without being asked ----------------------------------- +// +// `assess` answers "is one candidate ahead of the others", which is a question +// about the shape of the ranking. It is not the same question as "is this good +// enough to re-ground a session on", and a store of one context makes the +// difference plain: the leader is always uncontested there, so `clear` comes +// back on any query that matched a single word. +// +// So the floor below is absolute rather than relative. It asks how much of the +// request actually agreed with this context, and it lives here — beside +// `assess`, in host-neutral core — because nothing about it is specific to one +// host. Every bridge reads and writes the same `~/.neatcontext`; a rule about +// when routing may act unasked cannot be one host's private opinion. + +// How many independent parts of the request have to agree. +const MIN_AGREEING_TERMS = 2; + +// Where a hit has to land to count toward that floor. `FIELD_WEIGHTS` already +// rates `files` lowest because a knowledge folder's listing is incidental to +// what a context is *for*; a floor that counted filenames equally would throw +// that distinction away at the one moment it matters most, and "where is the +// deploy runbook?" would connect on two filenames and nothing else. +const INCIDENTAL_FIELDS = new Set(["files"]); + +export function normalizeRoutingText(text) { + return text.trim().toLowerCase().replace(/\s+/g, " "); +} + +// The request split the way the user wrote it, deduplicated: one entry per +// whitespace-separated run. +// +// This is the unit the floor counts, and it has to be, because a token is not +// one. `tokenize` deliberately expands a single `checkout-api` into +// `[checkout-api, checkout, api]` and a two-character CJK request into seven +// tokens — good for recall, but counting those as agreement means one word, or +// any CJK request at all, clears a floor meant to require two. +function queryTerms(query) { + return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); +} + +function agreeingTerms(candidate, query) { + const carried = new Set( + (candidate.matched ?? []).filter((term) => { + const fields = candidate.matchedFields?.[term]; + return !fields || fields.some((field) => !INCIDENTAL_FIELDS.has(field)); + }) + ); + if (carried.size === 0) { + return 0; + } + let agreeing = 0; + for (const term of queryTerms(query)) { + if (tokenize(term).some((token) => carried.has(token))) { + agreeing += 1; + } + } + return agreeing; +} + +// An alias is the one routing signal the user authored by hand, at the moment +// they were correcting a wrong route, so it may stand in for the term floor. +// Only when it is specific enough to be evidence, though: a one-word alias +// found inside a longer sentence is weaker than the rule it would be skipping, +// and `api`, `pr` or `lm` are exactly the aliases people write. A one-word +// alias therefore has to be the whole request; a longer one has to appear +// contiguously in the request's tokens. +// +// One word means one word the user typed, counted the way `queryTerms` counts +// the request — not the tokens the index derived from it. `tokenize` expands +// `checkout-api` into three and `user_id` into three, and reading that as a +// multi-word alias would reopen the bypass for every ticket id, service name +// and API version anyone is likely to register. +// +// It has to survive tokenizing as two, as well. `the api` is two words the user +// typed, but `tokenize` drops the stopword and leaves one, and a one-token +// contiguous check is just "does this word appear anywhere" — the very test the +// first floor exists to prevent. `the API`, `our PR`, `how LM works` are how +// people write these aliases down, so both floors have to hold. +function wordCount(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean).length; +} + +function matchesAlias(aliases, query) { + const normalized = normalizeRoutingText(query); + const queryTokens = tokenize(query); + return aliases.some((alias) => { + const aliasTokens = tokenize(alias); + if (aliasTokens.length === 0) { + return false; + } + if (wordCount(alias) < 2 || aliasTokens.length < 2) { + return normalizeRoutingText(alias) === normalized; + } + return queryTokens.some((_, start) => + aliasTokens.every((token, offset) => queryTokens[start + offset] === token) + ); + }); +} + +// Whether a leading candidate is strong enough to connect to without asking. +// `assess` has to have said `clear` first — this only decides whether the +// leader earned it. +export function isConfidentMatch(candidate, query, { aliases = [] } = {}) { + if (typeof query !== "string" || query.trim().length === 0) { + return false; + } + if (normalizeRoutingText(candidate?.name ?? "") === normalizeRoutingText(query)) { + return true; + } + return matchesAlias(aliases, query) || agreeingTerms(candidate, query) >= MIN_AGREEING_TERMS; +} + export function createRoutingIndex({ listFiles }) { let key = null; let index = null; @@ -109,14 +221,20 @@ export function createRoutingIndex({ listFiles }) { } const byId = new Map(contexts.map((context) => [context.id, context])); const now = new Date(); - const { connectedId = null } = options; + const { connectedId = null, limit = 5 } = options; // Past refusals are applied after ranking rather than folded into the // index: they change on their own schedule, and rebuilding the index every // time someone says no would throw away the cache for a multiplier. // // Re-sorted afterwards because a discount can change the order, and the // shortlist's whole meaning is that it is in order. - return rank(index, query, options) + // + // And cut to `limit` only after that, never before. `rank` slices on raw + // BM25, so a candidate that wins once its decline and familiarity + // multipliers are applied could be dropped before anything here ever saw + // it — silently, and most damagingly for `assess`, which would then report + // an uncontested leader because its rival had been cut. + return rank(index, query, { ...options, limit: Number.POSITIVE_INFINITY }) .map((result) => { const context = byId.get(result.id); return { @@ -126,9 +244,11 @@ export function createRoutingIndex({ listFiles }) { result.score * declineFactor(state, result.id, now) * familiarity(state, context, { connectedId, now }), - matched: result.matched + matched: result.matched, + matchedFields: result.matchedFields }; }) - .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)); + .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) + .slice(0, limit); }; } diff --git a/plugins/claude-code/neatcontext/src/core/routing-search.mjs b/plugins/claude-code/neatcontext/src/core/routing-search.mjs index ff660e4..22c1767 100644 --- a/plugins/claude-code/neatcontext/src/core/routing-search.mjs +++ b/plugins/claude-code/neatcontext/src/core/routing-search.mjs @@ -161,10 +161,18 @@ function addPosting(postings, token, id, field) { // Ranked candidates, best first, each with the query terms that put it there. // Those terms are the "why it matched" the session's model gets to read, and // they are the reason a caller can explain a route instead of asserting one. +// +// `matchedFields` says *where* each of those terms landed, which is the same +// distinction `FIELD_WEIGHTS` already makes and for the same reason: a hit in +// an alias the user wrote is evidence, and a hit in a filename picked up from a +// folder listing is a coincidence. Scoring weighs them apart; a caller deciding +// whether a match is strong enough to act on unasked needs to as well, and it +// cannot recover the field from the term alone. export function rank(index, query, { limit = 5 } = {}) { const terms = [...new Set(tokenize(query))]; const scores = new Map(); const matches = new Map(); + const landed = new Map(); for (const term of terms) { const byDocument = index.postings.get(term); @@ -183,11 +191,19 @@ export function rank(index, query, { limit = 5 } = {}) { } scores.set(id, (scores.get(id) ?? 0) + (idf * weighted) / (K1 + weighted)); matches.set(id, [...(matches.get(id) ?? []), term]); + const byTerm = landed.get(id) ?? new Map(); + byTerm.set(term, [...byField.keys()]); + landed.set(id, byTerm); } } return [...scores] - .map(([id, score]) => ({ id, score, matched: matches.get(id) })) + .map(([id, score]) => ({ + id, + score, + matched: matches.get(id), + matchedFields: Object.fromEntries(landed.get(id)) + })) .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) .slice(0, limit); } diff --git a/plugins/claude-code/neatcontext/src/core/routing.mjs b/plugins/claude-code/neatcontext/src/core/routing.mjs index 5c8da96..a0a401c 100644 --- a/plugins/claude-code/neatcontext/src/core/routing.mjs +++ b/plugins/claude-code/neatcontext/src/core/routing.mjs @@ -383,11 +383,18 @@ export function declineFactor(state, contextId, now = new Date()) { // Names are unique in a store, so this is exact; renaming a context forgets its // history, which for a hint of this size is a fair trade against threading an // id through five hosts' bridges. +// +// Routes the plugin made for itself are skipped. They are in the log because +// the log is the record of what happened, but they are not evidence about this +// user: reading them back would raise the multiplier of a context this machine +// chose on a keyword hit, making the same choice likelier next time and the one +// after — a mis-route that argues for itself. Only what a person or a model +// decided counts as familiarity. export function familiarity(state, context, { connectedId = null, now = new Date() } = {}) { const sticky = context.id === connectedId ? STICKY_BOOST : 1; let weight = 0; for (const decision of state.decisions ?? []) { - if (decision?.to !== context.name) continue; + if (decision?.to !== context.name || decision?.automatic === true) continue; const days = (now.getTime() - Date.parse(decision.at)) / DAY_MS; if (!Number.isFinite(days) || days < 0) continue; weight += 0.5 ** (days / FRECENCY_HALF_LIFE_DAYS); @@ -414,6 +421,13 @@ function pruneDeclines(declines, now) { // Every switch, with what it was routing away from and why. Thresholds and card // quality are guesses until this has something in it: manual selections are the // ground truth that says whether the derived lines actually route correctly. +// +// Which is why a route the plugin made for itself has to be marked `automatic` +// as it goes in. Left indistinguishable, machine routes accumulate at roughly +// one per new session and the log stops being able to answer the question it is +// kept for. `requested` does not carry that distinction — a model calling +// `use_context` in auto mode records `requested: false` too, and it was still a +// decision somebody made. export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); @@ -451,7 +465,12 @@ function describe(entry) { // an instruction from a context you are *not* connected to is still an // instruction sitting in the window, and it would bleed into how the session // answers on the context you are. -export function renderMenu(entries, { connectedId, mode } = {}) { +// +// It takes a `decision` for the same reason the shortlist does. A near-tie is +// something the plugin knows and the model cannot see, and a store too small +// for a shortlist is exactly where the full menu goes out instead — so leaving +// the note behind there means the one caller that most needs it never gets it. +export function renderMenu(entries, { connectedId, mode, decision } = {}) { if (mode === "manual" || entries.length === 0) { return null; } @@ -461,6 +480,10 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); + const tie = tieNote(decision); + if (tie) { + lines.push(tie); + } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } diff --git a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs index 99cad5a..adec3d5 100644 --- a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs +++ b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs @@ -10,7 +10,7 @@ // get_context instead of silently vanishing. import readline from "node:readline"; -import "./session.mjs"; +import { hasHostSessionId } from "./session.mjs"; import { readSelection } from "../core/local-state.mjs"; import { CONTEXT_MISSING_MESSAGE, @@ -33,8 +33,11 @@ import { sessionId, switchPolicy } from "../core/routing.mjs"; -import { assess, createRoutingIndex } from "../core/routing-candidates.mjs"; -import { tokenize } from "../core/routing-search.mjs"; +import { + assess, + createRoutingIndex, + isConfidentMatch +} from "../core/routing-candidates.mjs"; import { applySelection, resolveContext } from "../core/selection.mjs"; const SERVER_INFO = { name: "neatcontext", version: "0.3.4" }; @@ -92,13 +95,26 @@ const NOTHING_CONNECTED = // context came back as an offer to go and connect one by hand. The menu is // still what carries the mode-specific rules; these two only have to stop // contradicting it. -const NOTHING_CONNECTED_ROUTABLE = - `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + - "one is for. No safe automatic match was made for this call. Follow the routing " + - "rules below: connect a clear choice with `use_context`, ask when the choice is ambiguous, " + - "or say none covers the request. Do not ask the user to run a command to connect a context " + - "you can already name. If none covers the request, offer `/neatcontext:save` to make one out " + - "of this conversation. Until then, do not answer from general knowledge."; +// +// Whether automatic matching actually ran is threaded in rather than assumed. +// This text is reached with no query at all, on a stale selection, and when +// reading the store failed — and on each of those "no safe automatic match was +// made" would be the plugin telling the model that nothing in the store matched +// when nothing was ever compared. That is the same rule the comment above sets +// out for the handshake instructions: text that cannot know the current state +// must not assert it. +function nothingConnectedRoutable(assessed) { + return ( + `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + + "one is for. " + + (assessed ? "No safe automatic match was made for this call. " : "") + + "Follow the routing rules below: connect a clear choice with `use_context`, ask when the " + + "choice is ambiguous, or say none covers the request. Do not ask the user to run a command " + + "to connect a context you can already name. If none covers the request, offer " + + "`/neatcontext:save` to make one out of this conversation. Until then, do not answer from " + + "general knowledge." + ); +} const NOTHING_CONNECTED_ASK = `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + @@ -220,36 +236,47 @@ function jsonRpcResult(id, result) { // --- Context source: answers locally, from disk ------------------------------ -async function listAllContexts() { - return { contexts: await listContexts() }; -} - // Resolved per call, never fixed at startup: the user can create or save the // first context mid-session, and the next get_context has to stop telling them // they have none. The mode is read here for the same reason — it decides // whether a menu is about to follow this text, and therefore whether pointing // at a slash command is the honest answer or the one that breaks routing. -async function nothingConnectedText() { - const { contexts } = await listAllContexts().catch(() => ({ contexts: [] })); +// +// The routing pass this call already made is passed in rather than re-read, +// and it is also what says whether automatic matching ran at all. +async function nothingConnectedText(pass) { + const contexts = pass?.contexts ?? (await listContexts().catch(() => [])); if (contexts.length === 0) { return NOTHING_EXISTS; } - const mode = resolveMode(await readRouting().catch(() => ({ sessions: {} })), sessionId()); + const state = pass?.state ?? (await readRouting().catch(() => ({ sessions: {} }))); + const mode = resolveMode(state, sessionId()); if (mode === "manual") { return NOTHING_CONNECTED; } - return mode === "ask" ? NOTHING_CONNECTED_ASK : NOTHING_CONNECTED_ROUTABLE; + return mode === "ask" ? NOTHING_CONNECTED_ASK : nothingConnectedRoutable(pass?.assessed === true); } // The selected context, or null when nothing is selected. A selection // whose context was deleted out-of-band resolves to `missing` so get_context // can say what happened. -async function activeContext() { - const selection = await readSelection().catch(() => null); +// +// A context this call just connected is passed straight through: it was read +// out of the same listing a moment ago, and re-reading the selection file only +// to look it up again would be two disk hits to learn what is already in hand. +// The same listing answers the ordinary case, so a call that made one pass over +// the store makes exactly one. +async function activeContext(pass) { + if (pass?.connected) { + return { record: pass.connected }; + } + const selection = pass ? pass.selection : await readSelection().catch(() => null); if (!selection || selection.available === false) { return null; } - const record = await readContext(selection.contextId).catch(() => null); + const record = pass + ? (pass.contexts.find((context) => context.id === selection.contextId) ?? null) + : await readContext(selection.contextId).catch(() => null); return record ? { record } : { missing: true, name: selection.contextName }; } @@ -278,7 +305,7 @@ function dependsOnExtensions(message) { ); } -async function contextResponse(message, context) { +async function contextResponse(message, context, pass = null) { const { id, method, params } = message; if (id === undefined || id === null) { return null; // notification: nothing to answer @@ -303,7 +330,7 @@ async function contextResponse(message, context) { if (method === "tools/call" && params?.name === GET_CONTEXT_TOOL.name) { if (!context) { return jsonRpcResult(id, { - content: [{ type: "text", text: await nothingConnectedText() }], + content: [{ type: "text", text: await nothingConnectedText(pass) }], isError: false }); } @@ -353,39 +380,25 @@ const rankContexts = createRoutingIndex({ (await listKnowledgeFiles(record.knowledgeFolder, { limit: 60 })).files }); -async function routingMenu(query) { - const [{ contexts }, state] = await Promise.all([listAllContexts(), readRouting()]); - const selection = await readSelection().catch(() => null); - const options = { - connectedId: selection?.contextId ?? null, - mode: resolveMode(state, sessionId()) - }; +async function routingMenu(query, pass) { + const [contexts, state] = pass + ? [pass.contexts, pass.state] + : await Promise.all([listContexts(), readRouting()]); + const connectedId = pass + ? (pass.connected?.id ?? pass.selection?.contextId ?? null) + : ((await readSelection().catch(() => null))?.contextId ?? null); + const options = { connectedId, mode: resolveMode(state, sessionId()) }; const entries = menuEntries(contexts, state); - const shortlist = await shortlistFor(contexts, state, entries, query, options.connectedId); - return shortlist - ? renderShortlist(shortlist, { ...options, decision: assess(shortlist) }) - : renderMenu(entries, options); -} - -function normalizeRoutingText(value) { - return typeof value === "string" ? value.trim().toLowerCase().replace(/\s+/g, " ") : ""; -} - -function isConfidentMatch(entry, query) { - const normalizedQuery = normalizeRoutingText(query); - const queryTokens = tokenize(query); - const exact = - normalizeRoutingText(entry.name) === normalizedQuery || - entry.aliases.some((alias) => { - const aliasTokens = tokenize(alias); - return ( - aliasTokens.length > 0 && - queryTokens.some((_, start) => - aliasTokens.every((token, offset) => queryTokens[start + offset] === token) - ) - ); - }); - return exact || entry.matched.length >= 2; + const shortlist = await shortlistFor(contexts, state, entries, query, connectedId); + if (shortlist) { + return renderShortlist(shortlist, { ...options, decision: assess(shortlist) }); + } + // The full menu carries the tie the pass found, if it found one. A store + // below SHORTLIST_MIN_CONTEXTS never builds a shortlist, so this is the only + // way a near-tie the plugin already refused to act on reaches the model at + // all — and without it the plugin declines, says nothing about why, and the + // model picks one of the two anyway. + return renderMenu(entries, { ...options, decision: pass?.decision }); } // `get_context` is already the session asking the plugin to route this request. @@ -393,55 +406,99 @@ function isConfidentMatch(entry, query) { // the model to translate the returned shortlist into a second `use_context` // call. Existing connections are never changed by this shortcut: leaving a // context still needs the conversational follow-up judgment only the model has. -async function autoConnectClearMatch(query) { - if (typeof query !== "string" || query.trim().length === 0) { - return null; - } +// +// One pass over the store serves the whole call. What it loads — contexts, +// routing state, the ranking and its verdict — is what the menu and the +// nothing-connected text are built from a moment later, and re-reading it there +// made every `get_context` with a query hit the disk three times over for the +// same answer. +// +// The confidence rule itself lives in core beside `assess`, not here. Copilot +// is the first host to act on it and for now the only one; the other four share +// this machine's `~/.neatcontext` and keep the old behavior until they are +// wired up too, which is a staged rollout rather than a permanent split. +async function routingPass(query) { + const asked = typeof query === "string" && query.trim().length > 0; + const id = sessionId(); const selection = await readSelection().catch(() => null); - if (selection?.available === false || selection?.contextId) { - return null; - } - - const [{ contexts }, state] = await Promise.all([listAllContexts(), readRouting()]); - if (resolveMode(state, sessionId()) !== "auto") { - return null; - } + const [contexts, state] = await Promise.all([ + listContexts().catch(() => []), + readRouting().catch(() => ({ sessions: {}, cards: {} })) + ]); + const pass = { contexts, state, selection, connected: null, decision: null, assessed: false }; + + // Everything that stops the pass before it ranks anything, in one place, so + // `assessed` stays a statement about what actually happened. + // + // A selection carrying a `contextId` covers both "already connected" and + // "pointed at a context that is gone, and `readSelection` just cleared the + // file". The second is deliberate: a user whose context vanished should be + // told that, not silently re-grounded in a different one on the next answer. + if ( + !asked || + selection?.contextId || + contexts.length === 0 || + resolveMode(state, id) !== "auto" || + // Without a session id published by the host, one selection file is shared + // by every window open on this workspace. A model or a user calling + // `use_context` at least announces the switch; a keyword hit in one window + // would silently re-ground the conversation in the next. + !hasHostSessionId() + ) { + return pass; + } + + // Every candidate, not a top slice: the tie check is only as good as the + // field it can see. + // + // Everything from here is inside one guard. An auto-connection that cannot be + // made is a missed optimization, and that is all it may ever cost — unguarded, + // a home this process cannot write to turned every `get_context` in the + // session into a request that is never answered at all: the write rejected, + // `main` swallowed it, and nothing was written to stdout. + try { + const ranked = await rankContexts(contexts, state, query, { + limit: contexts.length, + connectedId: null + }); + pass.decision = assess(ranked); + pass.assessed = true; + if (pass.decision.verdict !== "clear") { + return pass; + } - const ranked = await rankContexts(contexts, state, query, { connectedId: null }); - const decision = assess(ranked); - const leader = decision.verdict === "clear" ? ranked[0] : null; - const target = leader && contexts.find((context) => context.id === leader.id); - if (!target) { - return null; - } + const leader = ranked[0]; + const target = contexts.find((context) => context.id === leader.id); + if (!target || !isConfidentMatch(leader, query, { aliases: aliasesOf(state, target.id) })) { + return pass; + } - const entry = { - ...menuEntries([target], state)[0], - matched: leader.matched - }; - if (!isConfidentMatch(entry, query)) { - return null; - } + // Mode was checked above to avoid ranking for nothing; this is the + // authority on whether the switch itself is allowed, declines included. + const policy = switchPolicy(state, { id, targetId: target.id, connectedId: null }); + if (!policy.allowed) { + return pass; + } - const policy = switchPolicy(state, { - id: sessionId(), - targetId: target.id, - connectedId: null - }); - if (!policy.allowed) { - return null; + await applySelection(target); + await noteDecision({ + sessionId: id, + from: null, + to: target.name, + mode: policy.mode, + reason: `clear query match: ${leader.matched.join(", ")}`, + requested: false, + automatic: true + }); + pass.connected = target; + } catch { + return pass; } + return pass; +} - await applySelection(target); - await noteDecision({ - sessionId: sessionId(), - from: null, - to: target.name, - mode: policy.mode, - reason: `clear query match: ${leader.matched.join(", ")}`, - requested: false - }); - return target.name; +function aliasesOf(state, contextId) { + return state.cards?.[contextId]?.aliases ?? []; } // A shortlist needs three things: a request to match against, enough contexts @@ -499,7 +556,7 @@ async function previewContext(id, target) { async function routingToolCall(message) { const { id, params } = message; const query = typeof params?.arguments?.context === "string" ? params.arguments.context : ""; - const { contexts } = await listAllContexts(); + const contexts = await listContexts(); const resolution = resolveContext(contexts, query); if (resolution.error) { return toolText( @@ -612,13 +669,19 @@ async function handleMessage(message) { return; } - const autoConnected = - message.method === "tools/call" && message.params?.name === GET_CONTEXT_TOOL.name - ? await autoConnectClearMatch(message.params?.arguments?.query) - : null; - const context = await activeContext(); - if (dependsOnExtensions(message)) { + const isGetContext = + message.method === "tools/call" && message.params?.name === GET_CONTEXT_TOOL.name; + const pass = isGetContext ? await routingPass(message.params?.arguments?.query) : null; + const context = await activeContext(pass); + // Extensions are deliberately not resolved on the turn that auto-connected. + // Resolving one starts the user's own server, and this is the one connection + // nobody asked for out loud: the announcement goes out first, and anything + // bound to the context starts on the next call that actually needs it. + if (dependsOnExtensions(message) && !pass?.connected) { await refreshExtensions(context); + } else if (pass?.connected) { + extensionTools = []; + extensionStatuses = []; } // An extension tool, proxied to the server the user bound for it. Answered @@ -631,7 +694,7 @@ async function handleMessage(message) { } } - const response = await contextResponse(message, context); + const response = await contextResponse(message, context, pass); if (message.method === "initialize" && response && response.result) { started = true; @@ -640,7 +703,7 @@ async function handleMessage(message) { } if (!isNotification && response) { - writeLine(await shapeResponse(message, response, autoConnected)); + writeLine(await shapeResponse(message, response, pass)); } } @@ -652,13 +715,13 @@ async function handleMessage(message) { // // The connection rule goes last, so it is the closest thing to the answer the // session is about to write — and it is the one part that is never omitted. -async function pluginNotes(query) { - const menu = await routingMenu(query); +async function pluginNotes(query, pass) { + const menu = await routingMenu(query, pass); return menu ? `${menu}\n\n${CONNECTION_RULE}` : CONNECTION_RULE; } -async function withNotes(response, place, query) { - const notes = await pluginNotes(query); +async function withNotes(response, place, query, pass) { + const notes = await pluginNotes(query, pass); if (place === "instructions") { const existing = response.result.instructions; return { @@ -702,7 +765,7 @@ function prependAutoConnection(response, contextName) { }; } -async function shapeResponse(message, response, autoConnected = null) { +async function shapeResponse(message, response, pass = null) { if (message.method === "initialize" && response.result) { return withNotes(response, "instructions"); } @@ -713,8 +776,8 @@ async function shapeResponse(message, response, autoConnected = null) { // The handshake has no request to match against, so only this path can // narrow the menu — which is also the path that is re-read every turn. return prependAutoConnection( - await withNotes(response, "content", message.params?.arguments?.query), - autoConnected + await withNotes(response, "content", message.params?.arguments?.query, pass), + pass?.connected?.name ?? null ); } return response; diff --git a/plugins/copilot/neatcontext/src/copilot/session.mjs b/plugins/copilot/neatcontext/src/copilot/session.mjs index 4dbe813..c5fb5f0 100644 --- a/plugins/copilot/neatcontext/src/copilot/session.mjs +++ b/plugins/copilot/neatcontext/src/copilot/session.mjs @@ -53,4 +53,20 @@ export function copilotSessionId() { ); } +// Whether this session has an identity of its own, rather than one it shares +// with every window open on the same folder. +// +// The workspace digest is a good enough fallback for remembering a choice +// someone made — a `use_context` call is announced, so the other window's user +// sees what happened and why. It is not good enough for a choice the plugin +// makes silently: routing acted on in one window would re-ground a conversation +// running in the next, mid-subject and unannounced. So anything that connects +// without being asked has to check this first. +export function hasHostSessionId() { + return ( + explicitId(process.env.NEATCONTEXT_SESSION_ID) !== null || + hostSessionId(process.env.COPILOT_AGENT_SESSION_ID) !== null + ); +} + configureSessionId(copilotSessionId); diff --git a/plugins/copilot/neatcontext/src/core/routing-candidates.mjs b/plugins/copilot/neatcontext/src/core/routing-candidates.mjs index b452f57..ce0eeb8 100644 --- a/plugins/copilot/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/copilot/neatcontext/src/core/routing-candidates.mjs @@ -19,7 +19,7 @@ // checking costs no extra reads. Only a real change pays for a rebuild. import { declineFactor, familiarity } from "./routing.mjs"; -import { buildIndex, rank } from "./routing-search.mjs"; +import { buildIndex, rank, tokenize } from "./routing-search.mjs"; // The fields, in the shape the scorer weighs. Aliases are joined into one // string rather than kept as a list because the scorer counts words, and a @@ -97,6 +97,118 @@ export function assess(ranked) { return { verdict: leaders.length > 1 ? "close" : "clear", leaders }; } +// --- acting on a match without being asked ----------------------------------- +// +// `assess` answers "is one candidate ahead of the others", which is a question +// about the shape of the ranking. It is not the same question as "is this good +// enough to re-ground a session on", and a store of one context makes the +// difference plain: the leader is always uncontested there, so `clear` comes +// back on any query that matched a single word. +// +// So the floor below is absolute rather than relative. It asks how much of the +// request actually agreed with this context, and it lives here — beside +// `assess`, in host-neutral core — because nothing about it is specific to one +// host. Every bridge reads and writes the same `~/.neatcontext`; a rule about +// when routing may act unasked cannot be one host's private opinion. + +// How many independent parts of the request have to agree. +const MIN_AGREEING_TERMS = 2; + +// Where a hit has to land to count toward that floor. `FIELD_WEIGHTS` already +// rates `files` lowest because a knowledge folder's listing is incidental to +// what a context is *for*; a floor that counted filenames equally would throw +// that distinction away at the one moment it matters most, and "where is the +// deploy runbook?" would connect on two filenames and nothing else. +const INCIDENTAL_FIELDS = new Set(["files"]); + +export function normalizeRoutingText(text) { + return text.trim().toLowerCase().replace(/\s+/g, " "); +} + +// The request split the way the user wrote it, deduplicated: one entry per +// whitespace-separated run. +// +// This is the unit the floor counts, and it has to be, because a token is not +// one. `tokenize` deliberately expands a single `checkout-api` into +// `[checkout-api, checkout, api]` and a two-character CJK request into seven +// tokens — good for recall, but counting those as agreement means one word, or +// any CJK request at all, clears a floor meant to require two. +function queryTerms(query) { + return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); +} + +function agreeingTerms(candidate, query) { + const carried = new Set( + (candidate.matched ?? []).filter((term) => { + const fields = candidate.matchedFields?.[term]; + return !fields || fields.some((field) => !INCIDENTAL_FIELDS.has(field)); + }) + ); + if (carried.size === 0) { + return 0; + } + let agreeing = 0; + for (const term of queryTerms(query)) { + if (tokenize(term).some((token) => carried.has(token))) { + agreeing += 1; + } + } + return agreeing; +} + +// An alias is the one routing signal the user authored by hand, at the moment +// they were correcting a wrong route, so it may stand in for the term floor. +// Only when it is specific enough to be evidence, though: a one-word alias +// found inside a longer sentence is weaker than the rule it would be skipping, +// and `api`, `pr` or `lm` are exactly the aliases people write. A one-word +// alias therefore has to be the whole request; a longer one has to appear +// contiguously in the request's tokens. +// +// One word means one word the user typed, counted the way `queryTerms` counts +// the request — not the tokens the index derived from it. `tokenize` expands +// `checkout-api` into three and `user_id` into three, and reading that as a +// multi-word alias would reopen the bypass for every ticket id, service name +// and API version anyone is likely to register. +// +// It has to survive tokenizing as two, as well. `the api` is two words the user +// typed, but `tokenize` drops the stopword and leaves one, and a one-token +// contiguous check is just "does this word appear anywhere" — the very test the +// first floor exists to prevent. `the API`, `our PR`, `how LM works` are how +// people write these aliases down, so both floors have to hold. +function wordCount(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean).length; +} + +function matchesAlias(aliases, query) { + const normalized = normalizeRoutingText(query); + const queryTokens = tokenize(query); + return aliases.some((alias) => { + const aliasTokens = tokenize(alias); + if (aliasTokens.length === 0) { + return false; + } + if (wordCount(alias) < 2 || aliasTokens.length < 2) { + return normalizeRoutingText(alias) === normalized; + } + return queryTokens.some((_, start) => + aliasTokens.every((token, offset) => queryTokens[start + offset] === token) + ); + }); +} + +// Whether a leading candidate is strong enough to connect to without asking. +// `assess` has to have said `clear` first — this only decides whether the +// leader earned it. +export function isConfidentMatch(candidate, query, { aliases = [] } = {}) { + if (typeof query !== "string" || query.trim().length === 0) { + return false; + } + if (normalizeRoutingText(candidate?.name ?? "") === normalizeRoutingText(query)) { + return true; + } + return matchesAlias(aliases, query) || agreeingTerms(candidate, query) >= MIN_AGREEING_TERMS; +} + export function createRoutingIndex({ listFiles }) { let key = null; let index = null; @@ -109,14 +221,20 @@ export function createRoutingIndex({ listFiles }) { } const byId = new Map(contexts.map((context) => [context.id, context])); const now = new Date(); - const { connectedId = null } = options; + const { connectedId = null, limit = 5 } = options; // Past refusals are applied after ranking rather than folded into the // index: they change on their own schedule, and rebuilding the index every // time someone says no would throw away the cache for a multiplier. // // Re-sorted afterwards because a discount can change the order, and the // shortlist's whole meaning is that it is in order. - return rank(index, query, options) + // + // And cut to `limit` only after that, never before. `rank` slices on raw + // BM25, so a candidate that wins once its decline and familiarity + // multipliers are applied could be dropped before anything here ever saw + // it — silently, and most damagingly for `assess`, which would then report + // an uncontested leader because its rival had been cut. + return rank(index, query, { ...options, limit: Number.POSITIVE_INFINITY }) .map((result) => { const context = byId.get(result.id); return { @@ -126,9 +244,11 @@ export function createRoutingIndex({ listFiles }) { result.score * declineFactor(state, result.id, now) * familiarity(state, context, { connectedId, now }), - matched: result.matched + matched: result.matched, + matchedFields: result.matchedFields }; }) - .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)); + .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) + .slice(0, limit); }; } diff --git a/plugins/copilot/neatcontext/src/core/routing-search.mjs b/plugins/copilot/neatcontext/src/core/routing-search.mjs index ff660e4..22c1767 100644 --- a/plugins/copilot/neatcontext/src/core/routing-search.mjs +++ b/plugins/copilot/neatcontext/src/core/routing-search.mjs @@ -161,10 +161,18 @@ function addPosting(postings, token, id, field) { // Ranked candidates, best first, each with the query terms that put it there. // Those terms are the "why it matched" the session's model gets to read, and // they are the reason a caller can explain a route instead of asserting one. +// +// `matchedFields` says *where* each of those terms landed, which is the same +// distinction `FIELD_WEIGHTS` already makes and for the same reason: a hit in +// an alias the user wrote is evidence, and a hit in a filename picked up from a +// folder listing is a coincidence. Scoring weighs them apart; a caller deciding +// whether a match is strong enough to act on unasked needs to as well, and it +// cannot recover the field from the term alone. export function rank(index, query, { limit = 5 } = {}) { const terms = [...new Set(tokenize(query))]; const scores = new Map(); const matches = new Map(); + const landed = new Map(); for (const term of terms) { const byDocument = index.postings.get(term); @@ -183,11 +191,19 @@ export function rank(index, query, { limit = 5 } = {}) { } scores.set(id, (scores.get(id) ?? 0) + (idf * weighted) / (K1 + weighted)); matches.set(id, [...(matches.get(id) ?? []), term]); + const byTerm = landed.get(id) ?? new Map(); + byTerm.set(term, [...byField.keys()]); + landed.set(id, byTerm); } } return [...scores] - .map(([id, score]) => ({ id, score, matched: matches.get(id) })) + .map(([id, score]) => ({ + id, + score, + matched: matches.get(id), + matchedFields: Object.fromEntries(landed.get(id)) + })) .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) .slice(0, limit); } diff --git a/plugins/copilot/neatcontext/src/core/routing.mjs b/plugins/copilot/neatcontext/src/core/routing.mjs index 5c8da96..a0a401c 100644 --- a/plugins/copilot/neatcontext/src/core/routing.mjs +++ b/plugins/copilot/neatcontext/src/core/routing.mjs @@ -383,11 +383,18 @@ export function declineFactor(state, contextId, now = new Date()) { // Names are unique in a store, so this is exact; renaming a context forgets its // history, which for a hint of this size is a fair trade against threading an // id through five hosts' bridges. +// +// Routes the plugin made for itself are skipped. They are in the log because +// the log is the record of what happened, but they are not evidence about this +// user: reading them back would raise the multiplier of a context this machine +// chose on a keyword hit, making the same choice likelier next time and the one +// after — a mis-route that argues for itself. Only what a person or a model +// decided counts as familiarity. export function familiarity(state, context, { connectedId = null, now = new Date() } = {}) { const sticky = context.id === connectedId ? STICKY_BOOST : 1; let weight = 0; for (const decision of state.decisions ?? []) { - if (decision?.to !== context.name) continue; + if (decision?.to !== context.name || decision?.automatic === true) continue; const days = (now.getTime() - Date.parse(decision.at)) / DAY_MS; if (!Number.isFinite(days) || days < 0) continue; weight += 0.5 ** (days / FRECENCY_HALF_LIFE_DAYS); @@ -414,6 +421,13 @@ function pruneDeclines(declines, now) { // Every switch, with what it was routing away from and why. Thresholds and card // quality are guesses until this has something in it: manual selections are the // ground truth that says whether the derived lines actually route correctly. +// +// Which is why a route the plugin made for itself has to be marked `automatic` +// as it goes in. Left indistinguishable, machine routes accumulate at roughly +// one per new session and the log stops being able to answer the question it is +// kept for. `requested` does not carry that distinction — a model calling +// `use_context` in auto mode records `requested: false` too, and it was still a +// decision somebody made. export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); @@ -451,7 +465,12 @@ function describe(entry) { // an instruction from a context you are *not* connected to is still an // instruction sitting in the window, and it would bleed into how the session // answers on the context you are. -export function renderMenu(entries, { connectedId, mode } = {}) { +// +// It takes a `decision` for the same reason the shortlist does. A near-tie is +// something the plugin knows and the model cannot see, and a store too small +// for a shortlist is exactly where the full menu goes out instead — so leaving +// the note behind there means the one caller that most needs it never gets it. +export function renderMenu(entries, { connectedId, mode, decision } = {}) { if (mode === "manual" || entries.length === 0) { return null; } @@ -461,6 +480,10 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); + const tie = tieNote(decision); + if (tie) { + lines.push(tie); + } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } diff --git a/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs b/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs index b452f57..ce0eeb8 100644 --- a/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs @@ -19,7 +19,7 @@ // checking costs no extra reads. Only a real change pays for a rebuild. import { declineFactor, familiarity } from "./routing.mjs"; -import { buildIndex, rank } from "./routing-search.mjs"; +import { buildIndex, rank, tokenize } from "./routing-search.mjs"; // The fields, in the shape the scorer weighs. Aliases are joined into one // string rather than kept as a list because the scorer counts words, and a @@ -97,6 +97,118 @@ export function assess(ranked) { return { verdict: leaders.length > 1 ? "close" : "clear", leaders }; } +// --- acting on a match without being asked ----------------------------------- +// +// `assess` answers "is one candidate ahead of the others", which is a question +// about the shape of the ranking. It is not the same question as "is this good +// enough to re-ground a session on", and a store of one context makes the +// difference plain: the leader is always uncontested there, so `clear` comes +// back on any query that matched a single word. +// +// So the floor below is absolute rather than relative. It asks how much of the +// request actually agreed with this context, and it lives here — beside +// `assess`, in host-neutral core — because nothing about it is specific to one +// host. Every bridge reads and writes the same `~/.neatcontext`; a rule about +// when routing may act unasked cannot be one host's private opinion. + +// How many independent parts of the request have to agree. +const MIN_AGREEING_TERMS = 2; + +// Where a hit has to land to count toward that floor. `FIELD_WEIGHTS` already +// rates `files` lowest because a knowledge folder's listing is incidental to +// what a context is *for*; a floor that counted filenames equally would throw +// that distinction away at the one moment it matters most, and "where is the +// deploy runbook?" would connect on two filenames and nothing else. +const INCIDENTAL_FIELDS = new Set(["files"]); + +export function normalizeRoutingText(text) { + return text.trim().toLowerCase().replace(/\s+/g, " "); +} + +// The request split the way the user wrote it, deduplicated: one entry per +// whitespace-separated run. +// +// This is the unit the floor counts, and it has to be, because a token is not +// one. `tokenize` deliberately expands a single `checkout-api` into +// `[checkout-api, checkout, api]` and a two-character CJK request into seven +// tokens — good for recall, but counting those as agreement means one word, or +// any CJK request at all, clears a floor meant to require two. +function queryTerms(query) { + return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); +} + +function agreeingTerms(candidate, query) { + const carried = new Set( + (candidate.matched ?? []).filter((term) => { + const fields = candidate.matchedFields?.[term]; + return !fields || fields.some((field) => !INCIDENTAL_FIELDS.has(field)); + }) + ); + if (carried.size === 0) { + return 0; + } + let agreeing = 0; + for (const term of queryTerms(query)) { + if (tokenize(term).some((token) => carried.has(token))) { + agreeing += 1; + } + } + return agreeing; +} + +// An alias is the one routing signal the user authored by hand, at the moment +// they were correcting a wrong route, so it may stand in for the term floor. +// Only when it is specific enough to be evidence, though: a one-word alias +// found inside a longer sentence is weaker than the rule it would be skipping, +// and `api`, `pr` or `lm` are exactly the aliases people write. A one-word +// alias therefore has to be the whole request; a longer one has to appear +// contiguously in the request's tokens. +// +// One word means one word the user typed, counted the way `queryTerms` counts +// the request — not the tokens the index derived from it. `tokenize` expands +// `checkout-api` into three and `user_id` into three, and reading that as a +// multi-word alias would reopen the bypass for every ticket id, service name +// and API version anyone is likely to register. +// +// It has to survive tokenizing as two, as well. `the api` is two words the user +// typed, but `tokenize` drops the stopword and leaves one, and a one-token +// contiguous check is just "does this word appear anywhere" — the very test the +// first floor exists to prevent. `the API`, `our PR`, `how LM works` are how +// people write these aliases down, so both floors have to hold. +function wordCount(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean).length; +} + +function matchesAlias(aliases, query) { + const normalized = normalizeRoutingText(query); + const queryTokens = tokenize(query); + return aliases.some((alias) => { + const aliasTokens = tokenize(alias); + if (aliasTokens.length === 0) { + return false; + } + if (wordCount(alias) < 2 || aliasTokens.length < 2) { + return normalizeRoutingText(alias) === normalized; + } + return queryTokens.some((_, start) => + aliasTokens.every((token, offset) => queryTokens[start + offset] === token) + ); + }); +} + +// Whether a leading candidate is strong enough to connect to without asking. +// `assess` has to have said `clear` first — this only decides whether the +// leader earned it. +export function isConfidentMatch(candidate, query, { aliases = [] } = {}) { + if (typeof query !== "string" || query.trim().length === 0) { + return false; + } + if (normalizeRoutingText(candidate?.name ?? "") === normalizeRoutingText(query)) { + return true; + } + return matchesAlias(aliases, query) || agreeingTerms(candidate, query) >= MIN_AGREEING_TERMS; +} + export function createRoutingIndex({ listFiles }) { let key = null; let index = null; @@ -109,14 +221,20 @@ export function createRoutingIndex({ listFiles }) { } const byId = new Map(contexts.map((context) => [context.id, context])); const now = new Date(); - const { connectedId = null } = options; + const { connectedId = null, limit = 5 } = options; // Past refusals are applied after ranking rather than folded into the // index: they change on their own schedule, and rebuilding the index every // time someone says no would throw away the cache for a multiplier. // // Re-sorted afterwards because a discount can change the order, and the // shortlist's whole meaning is that it is in order. - return rank(index, query, options) + // + // And cut to `limit` only after that, never before. `rank` slices on raw + // BM25, so a candidate that wins once its decline and familiarity + // multipliers are applied could be dropped before anything here ever saw + // it — silently, and most damagingly for `assess`, which would then report + // an uncontested leader because its rival had been cut. + return rank(index, query, { ...options, limit: Number.POSITIVE_INFINITY }) .map((result) => { const context = byId.get(result.id); return { @@ -126,9 +244,11 @@ export function createRoutingIndex({ listFiles }) { result.score * declineFactor(state, result.id, now) * familiarity(state, context, { connectedId, now }), - matched: result.matched + matched: result.matched, + matchedFields: result.matchedFields }; }) - .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)); + .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) + .slice(0, limit); }; } diff --git a/plugins/kimi-code/neatcontext/src/core/routing-search.mjs b/plugins/kimi-code/neatcontext/src/core/routing-search.mjs index ff660e4..22c1767 100644 --- a/plugins/kimi-code/neatcontext/src/core/routing-search.mjs +++ b/plugins/kimi-code/neatcontext/src/core/routing-search.mjs @@ -161,10 +161,18 @@ function addPosting(postings, token, id, field) { // Ranked candidates, best first, each with the query terms that put it there. // Those terms are the "why it matched" the session's model gets to read, and // they are the reason a caller can explain a route instead of asserting one. +// +// `matchedFields` says *where* each of those terms landed, which is the same +// distinction `FIELD_WEIGHTS` already makes and for the same reason: a hit in +// an alias the user wrote is evidence, and a hit in a filename picked up from a +// folder listing is a coincidence. Scoring weighs them apart; a caller deciding +// whether a match is strong enough to act on unasked needs to as well, and it +// cannot recover the field from the term alone. export function rank(index, query, { limit = 5 } = {}) { const terms = [...new Set(tokenize(query))]; const scores = new Map(); const matches = new Map(); + const landed = new Map(); for (const term of terms) { const byDocument = index.postings.get(term); @@ -183,11 +191,19 @@ export function rank(index, query, { limit = 5 } = {}) { } scores.set(id, (scores.get(id) ?? 0) + (idf * weighted) / (K1 + weighted)); matches.set(id, [...(matches.get(id) ?? []), term]); + const byTerm = landed.get(id) ?? new Map(); + byTerm.set(term, [...byField.keys()]); + landed.set(id, byTerm); } } return [...scores] - .map(([id, score]) => ({ id, score, matched: matches.get(id) })) + .map(([id, score]) => ({ + id, + score, + matched: matches.get(id), + matchedFields: Object.fromEntries(landed.get(id)) + })) .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) .slice(0, limit); } diff --git a/plugins/kimi-code/neatcontext/src/core/routing.mjs b/plugins/kimi-code/neatcontext/src/core/routing.mjs index 5c8da96..a0a401c 100644 --- a/plugins/kimi-code/neatcontext/src/core/routing.mjs +++ b/plugins/kimi-code/neatcontext/src/core/routing.mjs @@ -383,11 +383,18 @@ export function declineFactor(state, contextId, now = new Date()) { // Names are unique in a store, so this is exact; renaming a context forgets its // history, which for a hint of this size is a fair trade against threading an // id through five hosts' bridges. +// +// Routes the plugin made for itself are skipped. They are in the log because +// the log is the record of what happened, but they are not evidence about this +// user: reading them back would raise the multiplier of a context this machine +// chose on a keyword hit, making the same choice likelier next time and the one +// after — a mis-route that argues for itself. Only what a person or a model +// decided counts as familiarity. export function familiarity(state, context, { connectedId = null, now = new Date() } = {}) { const sticky = context.id === connectedId ? STICKY_BOOST : 1; let weight = 0; for (const decision of state.decisions ?? []) { - if (decision?.to !== context.name) continue; + if (decision?.to !== context.name || decision?.automatic === true) continue; const days = (now.getTime() - Date.parse(decision.at)) / DAY_MS; if (!Number.isFinite(days) || days < 0) continue; weight += 0.5 ** (days / FRECENCY_HALF_LIFE_DAYS); @@ -414,6 +421,13 @@ function pruneDeclines(declines, now) { // Every switch, with what it was routing away from and why. Thresholds and card // quality are guesses until this has something in it: manual selections are the // ground truth that says whether the derived lines actually route correctly. +// +// Which is why a route the plugin made for itself has to be marked `automatic` +// as it goes in. Left indistinguishable, machine routes accumulate at roughly +// one per new session and the log stops being able to answer the question it is +// kept for. `requested` does not carry that distinction — a model calling +// `use_context` in auto mode records `requested: false` too, and it was still a +// decision somebody made. export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); @@ -451,7 +465,12 @@ function describe(entry) { // an instruction from a context you are *not* connected to is still an // instruction sitting in the window, and it would bleed into how the session // answers on the context you are. -export function renderMenu(entries, { connectedId, mode } = {}) { +// +// It takes a `decision` for the same reason the shortlist does. A near-tie is +// something the plugin knows and the model cannot see, and a store too small +// for a shortlist is exactly where the full menu goes out instead — so leaving +// the note behind there means the one caller that most needs it never gets it. +export function renderMenu(entries, { connectedId, mode, decision } = {}) { if (mode === "manual" || entries.length === 0) { return null; } @@ -461,6 +480,10 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); + const tie = tieNote(decision); + if (tie) { + lines.push(tie); + } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } diff --git a/plugins/pi/neatcontext/src/core/routing-candidates.mjs b/plugins/pi/neatcontext/src/core/routing-candidates.mjs index b452f57..ce0eeb8 100644 --- a/plugins/pi/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/pi/neatcontext/src/core/routing-candidates.mjs @@ -19,7 +19,7 @@ // checking costs no extra reads. Only a real change pays for a rebuild. import { declineFactor, familiarity } from "./routing.mjs"; -import { buildIndex, rank } from "./routing-search.mjs"; +import { buildIndex, rank, tokenize } from "./routing-search.mjs"; // The fields, in the shape the scorer weighs. Aliases are joined into one // string rather than kept as a list because the scorer counts words, and a @@ -97,6 +97,118 @@ export function assess(ranked) { return { verdict: leaders.length > 1 ? "close" : "clear", leaders }; } +// --- acting on a match without being asked ----------------------------------- +// +// `assess` answers "is one candidate ahead of the others", which is a question +// about the shape of the ranking. It is not the same question as "is this good +// enough to re-ground a session on", and a store of one context makes the +// difference plain: the leader is always uncontested there, so `clear` comes +// back on any query that matched a single word. +// +// So the floor below is absolute rather than relative. It asks how much of the +// request actually agreed with this context, and it lives here — beside +// `assess`, in host-neutral core — because nothing about it is specific to one +// host. Every bridge reads and writes the same `~/.neatcontext`; a rule about +// when routing may act unasked cannot be one host's private opinion. + +// How many independent parts of the request have to agree. +const MIN_AGREEING_TERMS = 2; + +// Where a hit has to land to count toward that floor. `FIELD_WEIGHTS` already +// rates `files` lowest because a knowledge folder's listing is incidental to +// what a context is *for*; a floor that counted filenames equally would throw +// that distinction away at the one moment it matters most, and "where is the +// deploy runbook?" would connect on two filenames and nothing else. +const INCIDENTAL_FIELDS = new Set(["files"]); + +export function normalizeRoutingText(text) { + return text.trim().toLowerCase().replace(/\s+/g, " "); +} + +// The request split the way the user wrote it, deduplicated: one entry per +// whitespace-separated run. +// +// This is the unit the floor counts, and it has to be, because a token is not +// one. `tokenize` deliberately expands a single `checkout-api` into +// `[checkout-api, checkout, api]` and a two-character CJK request into seven +// tokens — good for recall, but counting those as agreement means one word, or +// any CJK request at all, clears a floor meant to require two. +function queryTerms(query) { + return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); +} + +function agreeingTerms(candidate, query) { + const carried = new Set( + (candidate.matched ?? []).filter((term) => { + const fields = candidate.matchedFields?.[term]; + return !fields || fields.some((field) => !INCIDENTAL_FIELDS.has(field)); + }) + ); + if (carried.size === 0) { + return 0; + } + let agreeing = 0; + for (const term of queryTerms(query)) { + if (tokenize(term).some((token) => carried.has(token))) { + agreeing += 1; + } + } + return agreeing; +} + +// An alias is the one routing signal the user authored by hand, at the moment +// they were correcting a wrong route, so it may stand in for the term floor. +// Only when it is specific enough to be evidence, though: a one-word alias +// found inside a longer sentence is weaker than the rule it would be skipping, +// and `api`, `pr` or `lm` are exactly the aliases people write. A one-word +// alias therefore has to be the whole request; a longer one has to appear +// contiguously in the request's tokens. +// +// One word means one word the user typed, counted the way `queryTerms` counts +// the request — not the tokens the index derived from it. `tokenize` expands +// `checkout-api` into three and `user_id` into three, and reading that as a +// multi-word alias would reopen the bypass for every ticket id, service name +// and API version anyone is likely to register. +// +// It has to survive tokenizing as two, as well. `the api` is two words the user +// typed, but `tokenize` drops the stopword and leaves one, and a one-token +// contiguous check is just "does this word appear anywhere" — the very test the +// first floor exists to prevent. `the API`, `our PR`, `how LM works` are how +// people write these aliases down, so both floors have to hold. +function wordCount(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean).length; +} + +function matchesAlias(aliases, query) { + const normalized = normalizeRoutingText(query); + const queryTokens = tokenize(query); + return aliases.some((alias) => { + const aliasTokens = tokenize(alias); + if (aliasTokens.length === 0) { + return false; + } + if (wordCount(alias) < 2 || aliasTokens.length < 2) { + return normalizeRoutingText(alias) === normalized; + } + return queryTokens.some((_, start) => + aliasTokens.every((token, offset) => queryTokens[start + offset] === token) + ); + }); +} + +// Whether a leading candidate is strong enough to connect to without asking. +// `assess` has to have said `clear` first — this only decides whether the +// leader earned it. +export function isConfidentMatch(candidate, query, { aliases = [] } = {}) { + if (typeof query !== "string" || query.trim().length === 0) { + return false; + } + if (normalizeRoutingText(candidate?.name ?? "") === normalizeRoutingText(query)) { + return true; + } + return matchesAlias(aliases, query) || agreeingTerms(candidate, query) >= MIN_AGREEING_TERMS; +} + export function createRoutingIndex({ listFiles }) { let key = null; let index = null; @@ -109,14 +221,20 @@ export function createRoutingIndex({ listFiles }) { } const byId = new Map(contexts.map((context) => [context.id, context])); const now = new Date(); - const { connectedId = null } = options; + const { connectedId = null, limit = 5 } = options; // Past refusals are applied after ranking rather than folded into the // index: they change on their own schedule, and rebuilding the index every // time someone says no would throw away the cache for a multiplier. // // Re-sorted afterwards because a discount can change the order, and the // shortlist's whole meaning is that it is in order. - return rank(index, query, options) + // + // And cut to `limit` only after that, never before. `rank` slices on raw + // BM25, so a candidate that wins once its decline and familiarity + // multipliers are applied could be dropped before anything here ever saw + // it — silently, and most damagingly for `assess`, which would then report + // an uncontested leader because its rival had been cut. + return rank(index, query, { ...options, limit: Number.POSITIVE_INFINITY }) .map((result) => { const context = byId.get(result.id); return { @@ -126,9 +244,11 @@ export function createRoutingIndex({ listFiles }) { result.score * declineFactor(state, result.id, now) * familiarity(state, context, { connectedId, now }), - matched: result.matched + matched: result.matched, + matchedFields: result.matchedFields }; }) - .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)); + .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) + .slice(0, limit); }; } diff --git a/plugins/pi/neatcontext/src/core/routing-search.mjs b/plugins/pi/neatcontext/src/core/routing-search.mjs index ff660e4..22c1767 100644 --- a/plugins/pi/neatcontext/src/core/routing-search.mjs +++ b/plugins/pi/neatcontext/src/core/routing-search.mjs @@ -161,10 +161,18 @@ function addPosting(postings, token, id, field) { // Ranked candidates, best first, each with the query terms that put it there. // Those terms are the "why it matched" the session's model gets to read, and // they are the reason a caller can explain a route instead of asserting one. +// +// `matchedFields` says *where* each of those terms landed, which is the same +// distinction `FIELD_WEIGHTS` already makes and for the same reason: a hit in +// an alias the user wrote is evidence, and a hit in a filename picked up from a +// folder listing is a coincidence. Scoring weighs them apart; a caller deciding +// whether a match is strong enough to act on unasked needs to as well, and it +// cannot recover the field from the term alone. export function rank(index, query, { limit = 5 } = {}) { const terms = [...new Set(tokenize(query))]; const scores = new Map(); const matches = new Map(); + const landed = new Map(); for (const term of terms) { const byDocument = index.postings.get(term); @@ -183,11 +191,19 @@ export function rank(index, query, { limit = 5 } = {}) { } scores.set(id, (scores.get(id) ?? 0) + (idf * weighted) / (K1 + weighted)); matches.set(id, [...(matches.get(id) ?? []), term]); + const byTerm = landed.get(id) ?? new Map(); + byTerm.set(term, [...byField.keys()]); + landed.set(id, byTerm); } } return [...scores] - .map(([id, score]) => ({ id, score, matched: matches.get(id) })) + .map(([id, score]) => ({ + id, + score, + matched: matches.get(id), + matchedFields: Object.fromEntries(landed.get(id)) + })) .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) .slice(0, limit); } diff --git a/plugins/pi/neatcontext/src/core/routing.mjs b/plugins/pi/neatcontext/src/core/routing.mjs index 5c8da96..a0a401c 100644 --- a/plugins/pi/neatcontext/src/core/routing.mjs +++ b/plugins/pi/neatcontext/src/core/routing.mjs @@ -383,11 +383,18 @@ export function declineFactor(state, contextId, now = new Date()) { // Names are unique in a store, so this is exact; renaming a context forgets its // history, which for a hint of this size is a fair trade against threading an // id through five hosts' bridges. +// +// Routes the plugin made for itself are skipped. They are in the log because +// the log is the record of what happened, but they are not evidence about this +// user: reading them back would raise the multiplier of a context this machine +// chose on a keyword hit, making the same choice likelier next time and the one +// after — a mis-route that argues for itself. Only what a person or a model +// decided counts as familiarity. export function familiarity(state, context, { connectedId = null, now = new Date() } = {}) { const sticky = context.id === connectedId ? STICKY_BOOST : 1; let weight = 0; for (const decision of state.decisions ?? []) { - if (decision?.to !== context.name) continue; + if (decision?.to !== context.name || decision?.automatic === true) continue; const days = (now.getTime() - Date.parse(decision.at)) / DAY_MS; if (!Number.isFinite(days) || days < 0) continue; weight += 0.5 ** (days / FRECENCY_HALF_LIFE_DAYS); @@ -414,6 +421,13 @@ function pruneDeclines(declines, now) { // Every switch, with what it was routing away from and why. Thresholds and card // quality are guesses until this has something in it: manual selections are the // ground truth that says whether the derived lines actually route correctly. +// +// Which is why a route the plugin made for itself has to be marked `automatic` +// as it goes in. Left indistinguishable, machine routes accumulate at roughly +// one per new session and the log stops being able to answer the question it is +// kept for. `requested` does not carry that distinction — a model calling +// `use_context` in auto mode records `requested: false` too, and it was still a +// decision somebody made. export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); @@ -451,7 +465,12 @@ function describe(entry) { // an instruction from a context you are *not* connected to is still an // instruction sitting in the window, and it would bleed into how the session // answers on the context you are. -export function renderMenu(entries, { connectedId, mode } = {}) { +// +// It takes a `decision` for the same reason the shortlist does. A near-tie is +// something the plugin knows and the model cannot see, and a store too small +// for a shortlist is exactly where the full menu goes out instead — so leaving +// the note behind there means the one caller that most needs it never gets it. +export function renderMenu(entries, { connectedId, mode, decision } = {}) { if (mode === "manual" || entries.length === 0) { return null; } @@ -461,6 +480,10 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); + const tie = tieNote(decision); + if (tie) { + lines.push(tie); + } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } diff --git a/shared/core/routing-candidates.mjs b/shared/core/routing-candidates.mjs index b452f57..ce0eeb8 100644 --- a/shared/core/routing-candidates.mjs +++ b/shared/core/routing-candidates.mjs @@ -19,7 +19,7 @@ // checking costs no extra reads. Only a real change pays for a rebuild. import { declineFactor, familiarity } from "./routing.mjs"; -import { buildIndex, rank } from "./routing-search.mjs"; +import { buildIndex, rank, tokenize } from "./routing-search.mjs"; // The fields, in the shape the scorer weighs. Aliases are joined into one // string rather than kept as a list because the scorer counts words, and a @@ -97,6 +97,118 @@ export function assess(ranked) { return { verdict: leaders.length > 1 ? "close" : "clear", leaders }; } +// --- acting on a match without being asked ----------------------------------- +// +// `assess` answers "is one candidate ahead of the others", which is a question +// about the shape of the ranking. It is not the same question as "is this good +// enough to re-ground a session on", and a store of one context makes the +// difference plain: the leader is always uncontested there, so `clear` comes +// back on any query that matched a single word. +// +// So the floor below is absolute rather than relative. It asks how much of the +// request actually agreed with this context, and it lives here — beside +// `assess`, in host-neutral core — because nothing about it is specific to one +// host. Every bridge reads and writes the same `~/.neatcontext`; a rule about +// when routing may act unasked cannot be one host's private opinion. + +// How many independent parts of the request have to agree. +const MIN_AGREEING_TERMS = 2; + +// Where a hit has to land to count toward that floor. `FIELD_WEIGHTS` already +// rates `files` lowest because a knowledge folder's listing is incidental to +// what a context is *for*; a floor that counted filenames equally would throw +// that distinction away at the one moment it matters most, and "where is the +// deploy runbook?" would connect on two filenames and nothing else. +const INCIDENTAL_FIELDS = new Set(["files"]); + +export function normalizeRoutingText(text) { + return text.trim().toLowerCase().replace(/\s+/g, " "); +} + +// The request split the way the user wrote it, deduplicated: one entry per +// whitespace-separated run. +// +// This is the unit the floor counts, and it has to be, because a token is not +// one. `tokenize` deliberately expands a single `checkout-api` into +// `[checkout-api, checkout, api]` and a two-character CJK request into seven +// tokens — good for recall, but counting those as agreement means one word, or +// any CJK request at all, clears a floor meant to require two. +function queryTerms(query) { + return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); +} + +function agreeingTerms(candidate, query) { + const carried = new Set( + (candidate.matched ?? []).filter((term) => { + const fields = candidate.matchedFields?.[term]; + return !fields || fields.some((field) => !INCIDENTAL_FIELDS.has(field)); + }) + ); + if (carried.size === 0) { + return 0; + } + let agreeing = 0; + for (const term of queryTerms(query)) { + if (tokenize(term).some((token) => carried.has(token))) { + agreeing += 1; + } + } + return agreeing; +} + +// An alias is the one routing signal the user authored by hand, at the moment +// they were correcting a wrong route, so it may stand in for the term floor. +// Only when it is specific enough to be evidence, though: a one-word alias +// found inside a longer sentence is weaker than the rule it would be skipping, +// and `api`, `pr` or `lm` are exactly the aliases people write. A one-word +// alias therefore has to be the whole request; a longer one has to appear +// contiguously in the request's tokens. +// +// One word means one word the user typed, counted the way `queryTerms` counts +// the request — not the tokens the index derived from it. `tokenize` expands +// `checkout-api` into three and `user_id` into three, and reading that as a +// multi-word alias would reopen the bypass for every ticket id, service name +// and API version anyone is likely to register. +// +// It has to survive tokenizing as two, as well. `the api` is two words the user +// typed, but `tokenize` drops the stopword and leaves one, and a one-token +// contiguous check is just "does this word appear anywhere" — the very test the +// first floor exists to prevent. `the API`, `our PR`, `how LM works` are how +// people write these aliases down, so both floors have to hold. +function wordCount(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean).length; +} + +function matchesAlias(aliases, query) { + const normalized = normalizeRoutingText(query); + const queryTokens = tokenize(query); + return aliases.some((alias) => { + const aliasTokens = tokenize(alias); + if (aliasTokens.length === 0) { + return false; + } + if (wordCount(alias) < 2 || aliasTokens.length < 2) { + return normalizeRoutingText(alias) === normalized; + } + return queryTokens.some((_, start) => + aliasTokens.every((token, offset) => queryTokens[start + offset] === token) + ); + }); +} + +// Whether a leading candidate is strong enough to connect to without asking. +// `assess` has to have said `clear` first — this only decides whether the +// leader earned it. +export function isConfidentMatch(candidate, query, { aliases = [] } = {}) { + if (typeof query !== "string" || query.trim().length === 0) { + return false; + } + if (normalizeRoutingText(candidate?.name ?? "") === normalizeRoutingText(query)) { + return true; + } + return matchesAlias(aliases, query) || agreeingTerms(candidate, query) >= MIN_AGREEING_TERMS; +} + export function createRoutingIndex({ listFiles }) { let key = null; let index = null; @@ -109,14 +221,20 @@ export function createRoutingIndex({ listFiles }) { } const byId = new Map(contexts.map((context) => [context.id, context])); const now = new Date(); - const { connectedId = null } = options; + const { connectedId = null, limit = 5 } = options; // Past refusals are applied after ranking rather than folded into the // index: they change on their own schedule, and rebuilding the index every // time someone says no would throw away the cache for a multiplier. // // Re-sorted afterwards because a discount can change the order, and the // shortlist's whole meaning is that it is in order. - return rank(index, query, options) + // + // And cut to `limit` only after that, never before. `rank` slices on raw + // BM25, so a candidate that wins once its decline and familiarity + // multipliers are applied could be dropped before anything here ever saw + // it — silently, and most damagingly for `assess`, which would then report + // an uncontested leader because its rival had been cut. + return rank(index, query, { ...options, limit: Number.POSITIVE_INFINITY }) .map((result) => { const context = byId.get(result.id); return { @@ -126,9 +244,11 @@ export function createRoutingIndex({ listFiles }) { result.score * declineFactor(state, result.id, now) * familiarity(state, context, { connectedId, now }), - matched: result.matched + matched: result.matched, + matchedFields: result.matchedFields }; }) - .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)); + .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) + .slice(0, limit); }; } diff --git a/shared/core/routing-search.mjs b/shared/core/routing-search.mjs index ff660e4..22c1767 100644 --- a/shared/core/routing-search.mjs +++ b/shared/core/routing-search.mjs @@ -161,10 +161,18 @@ function addPosting(postings, token, id, field) { // Ranked candidates, best first, each with the query terms that put it there. // Those terms are the "why it matched" the session's model gets to read, and // they are the reason a caller can explain a route instead of asserting one. +// +// `matchedFields` says *where* each of those terms landed, which is the same +// distinction `FIELD_WEIGHTS` already makes and for the same reason: a hit in +// an alias the user wrote is evidence, and a hit in a filename picked up from a +// folder listing is a coincidence. Scoring weighs them apart; a caller deciding +// whether a match is strong enough to act on unasked needs to as well, and it +// cannot recover the field from the term alone. export function rank(index, query, { limit = 5 } = {}) { const terms = [...new Set(tokenize(query))]; const scores = new Map(); const matches = new Map(); + const landed = new Map(); for (const term of terms) { const byDocument = index.postings.get(term); @@ -183,11 +191,19 @@ export function rank(index, query, { limit = 5 } = {}) { } scores.set(id, (scores.get(id) ?? 0) + (idf * weighted) / (K1 + weighted)); matches.set(id, [...(matches.get(id) ?? []), term]); + const byTerm = landed.get(id) ?? new Map(); + byTerm.set(term, [...byField.keys()]); + landed.set(id, byTerm); } } return [...scores] - .map(([id, score]) => ({ id, score, matched: matches.get(id) })) + .map(([id, score]) => ({ + id, + score, + matched: matches.get(id), + matchedFields: Object.fromEntries(landed.get(id)) + })) .sort((left, right) => right.score - left.score || left.id.localeCompare(right.id)) .slice(0, limit); } diff --git a/shared/core/routing.mjs b/shared/core/routing.mjs index 5c8da96..a0a401c 100644 --- a/shared/core/routing.mjs +++ b/shared/core/routing.mjs @@ -383,11 +383,18 @@ export function declineFactor(state, contextId, now = new Date()) { // Names are unique in a store, so this is exact; renaming a context forgets its // history, which for a hint of this size is a fair trade against threading an // id through five hosts' bridges. +// +// Routes the plugin made for itself are skipped. They are in the log because +// the log is the record of what happened, but they are not evidence about this +// user: reading them back would raise the multiplier of a context this machine +// chose on a keyword hit, making the same choice likelier next time and the one +// after — a mis-route that argues for itself. Only what a person or a model +// decided counts as familiarity. export function familiarity(state, context, { connectedId = null, now = new Date() } = {}) { const sticky = context.id === connectedId ? STICKY_BOOST : 1; let weight = 0; for (const decision of state.decisions ?? []) { - if (decision?.to !== context.name) continue; + if (decision?.to !== context.name || decision?.automatic === true) continue; const days = (now.getTime() - Date.parse(decision.at)) / DAY_MS; if (!Number.isFinite(days) || days < 0) continue; weight += 0.5 ** (days / FRECENCY_HALF_LIFE_DAYS); @@ -414,6 +421,13 @@ function pruneDeclines(declines, now) { // Every switch, with what it was routing away from and why. Thresholds and card // quality are guesses until this has something in it: manual selections are the // ground truth that says whether the derived lines actually route correctly. +// +// Which is why a route the plugin made for itself has to be marked `automatic` +// as it goes in. Left indistinguishable, machine routes accumulate at roughly +// one per new session and the log stops being able to answer the question it is +// kept for. `requested` does not carry that distinction — a model calling +// `use_context` in auto mode records `requested: false` too, and it was still a +// decision somebody made. export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); @@ -451,7 +465,12 @@ function describe(entry) { // an instruction from a context you are *not* connected to is still an // instruction sitting in the window, and it would bleed into how the session // answers on the context you are. -export function renderMenu(entries, { connectedId, mode } = {}) { +// +// It takes a `decision` for the same reason the shortlist does. A near-tie is +// something the plugin knows and the model cannot see, and a store too small +// for a shortlist is exactly where the full menu goes out instead — so leaving +// the note behind there means the one caller that most needs it never gets it. +export function renderMenu(entries, { connectedId, mode, decision } = {}) { if (mode === "manual" || entries.length === 0) { return null; } @@ -461,6 +480,10 @@ export function renderMenu(entries, { connectedId, mode } = {}) { lines.push(`- **${entry.name}**${marker} — ${describe(entry)}`); } lines.push(""); + const tie = tieNote(decision); + if (tie) { + lines.push(tie); + } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); } diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index f9270c6..a84bfe7 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -714,8 +714,14 @@ test("Copilot MCP bridge serves Contexts and routing locally", async (t) => { assert.match(empty.result.content[0].text, /No NeatContext Context is connected/); // A context exists and routing is on, so the answer leads with the route this // session can take itself rather than with a command for the user to type. - assert.match(empty.result.content[0].text, /No safe automatic match was made for this call/); assert.match(empty.result.content[0].text, /connect a clear choice with `use_context`/); + // Nothing was passed to match against, so nothing was matched. The text must + // not report a decision that was never made. + assert.doesNotMatch( + empty.result.content[0].text, + /No safe automatic match was made for this call/, + "with no query, no matching ran — saying otherwise is evidence the model acts on" + ); // Ask mode refuses an unrequested switch. It is set explicitly rather than // assumed, so this keeps testing ask mode whatever the default becomes. @@ -871,7 +877,6 @@ test("Copilot narrows the routing menu to the request", async (t) => { for (const [name, useWhen] of corpus) { await createContext(home, name, { sessionId: "copilot-shortlist", useWhen }); } - await runNode(cli, ["mode", "ask"], { env }); const session = rpcSession(env); sessions.push(session); @@ -881,9 +886,12 @@ test("Copilot narrows the routing menu to the request", async (t) => { const getContext = tools.result.tools.find((tool) => tool.name === "get_context"); assert.equal(getContext.inputSchema.properties.query.type, "string"); - const matched = await session.call( - toolCall(3, "get_context", { query: "why is checkout throwing 5xx" }) - ); + // Auto mode, which is the default and the only one most users ever see. The + // request is one word, so it narrows the menu without earning a connection: + // `checkout-api` becomes three tokens but it is still one thing the user + // typed, and one is below the floor for acting unasked. + const matched = await session.call(toolCall(3, "get_context", { query: "checkout-api" })); + assert.doesNotMatch(matched.result.content[0].text, /Automatically connected/); assert.match(matched.result.content[0].text, /## Contexts that match what the user just asked/); assert.match(matched.result.content[0].text, /INC-1001 checkout/); assert.ok(!matched.result.content[0].text.includes("Docker container")); @@ -902,6 +910,50 @@ test("Copilot narrows the routing menu to the request", async (t) => { assert.match(unmatched.result.content[0].text, /Docker container/); }); +// The interaction auto-connect actually introduced, which nothing else covers: +// a store large enough for a shortlist, connected mid-call, so the menu below +// the answer is built with a `connectedId` that did not exist when the call +// arrived — and STICKY_BOOST now applies to the context this same call chose. +test("Copilot shows the shortlist against the context it just auto-connected", async (t) => { + const home = await isolatedHome("neatcontext-copilot-shortlist-auto-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-shortlist-auto"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + + const corpus = [ + ["INC-1001 checkout", "checkout-api 5xx from pgbouncer pool exhaustion"], + ["Queue lag", "order-events partition lag and consumer rebalancing"], + ["Codex design", "Codex CLI plugin design and marketplace packaging"], + ["Kimi plugin", "Kimi Code manifests, skills and commands"], + ["Evidence", "conversation evidence and transcript adapters"], + ["Refunds", "refunds and chargebacks"], + ["Docker container", "Ubuntu container with SSH"], + ["Marketplace config", "switching the marketplace source"], + ["Session drift", "bridge session and thread drift"] + ]; + for (const [name, useWhen] of corpus) { + await createContext(home, name, { sessionId, useWhen }); + } + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const response = await session.call( + toolCall(2, "get_context", { query: "checkout-api 5xx from pgbouncer pool exhaustion" }) + ); + const text = response.result.content[0].text; + assert.match(text, /Automatically connected "INC-1001 checkout"/); + assert.match(text, /## Contexts that match what the user just asked/); + // Named as connected in the very menu the connecting call returned, so the + // model is not invited to route again to where it already is. + assert.match(text, /\*\*INC-1001 checkout\*\* \*\*\(connected\)\*\*/); + assert.match(text, /Routing is on \(auto\)/); +}); + test("Copilot get_context auto-connects a uniquely clear first context in auto mode", async (t) => { const home = await isolatedHome("neatcontext-copilot-auto-connect-"); const sessions = []; @@ -949,11 +1001,144 @@ test("Copilot get_context auto-connects a uniquely clear first context in auto m to: "LM coordination", mode: "auto", reason: "clear query match: lm, coordination, implemented, windows, servicemanager", - requested: false + requested: false, + automatic: true } ]); }); +// The gate is a floor on how much of the request agreed, and these are the four +// ways a match can look like two terms without being two terms. +test("Copilot get_context does not auto-connect on evidence weaker than it looks", async (t) => { + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + + const ask = async (label, build, query) => { + const home = await isolatedHome(`neatcontext-copilot-gate-${label}-`); + const sessionId = `copilot-gate-${label}`; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await build(home, sessionId, env); + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + const response = await session.call(toolCall(2, "get_context", { query })); + return response.result.content[0].text; + }; + + // One hyphenated word tokenizes into three, but the user typed one thing. + const hyphenated = await ask( + "hyphenated", + (home, sessionId) => + createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }), + "checkout-api" + ); + assert.doesNotMatch(hyphenated, /Automatically connected/); + assert.match(hyphenated, /Checkout incident/); + + // A CJK run contributes its characters and their pairs — seven tokens for two + // characters, and still one thing asked about. + const cjk = await ask( + "cjk", + (home, sessionId) => + createContext(home, "订单系统", { sessionId, useWhen: "订单延迟与消费者重平衡" }), + "订单延迟" + ); + assert.doesNotMatch(cjk, /Automatically connected/); + + // Two filenames from a knowledge folder are not two statements about what a + // context is for. `FIELD_WEIGHTS` already says so; the floor has to as well. + const filenames = await ask( + "filenames", + async (home, sessionId) => { + await knowledgeFolder(home, { "deploy.md": "# deploy\n", "runbook.md": "# runbook\n" }); + await createContext(home, "Payments", { sessionId, useWhen: "settlement reconciliation" }); + }, + "where is the deploy runbook?" + ); + assert.doesNotMatch(filenames, /Automatically connected/); + + // A one-word alias inside an ordinary sentence is weaker evidence than the + // floor it would be skipping. + const alias = await ask( + "alias", + async (home, sessionId, env) => { + await createContext(home, "Windows notes", { sessionId, useWhen: "Windows build notes" }); + const added = await runNode(cli, ["alias", "Windows notes", "--called", "windows"], { env }); + assert.equal(added.code, 0); + }, + "how do I install Docker on Windows?" + ); + assert.doesNotMatch(alias, /Automatically connected/); + assert.match(alias, /Windows notes/); +}); + +// Auto-connect is the one route nobody announces before it happens, so it needs +// a session it cannot leak out of. Without an id from the host, one selection +// file is shared by every window open on the same folder. +test("Copilot get_context does not auto-connect without a host session id", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-shared-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + await createContext(home, "Checkout incident", { + sessionId: "seed", + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + + const env = { ...home.env, NEATCONTEXT_SESSION_ID: "", COPILOT_AGENT_SESSION_ID: "" }; + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const response = await session.call( + toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); +}); + +// A home it cannot write to must cost the caller an optimization, never the +// answer. Unguarded, the write rejected, the rejection was swallowed, and the +// request was never answered at all — as was every later one in the session. +test("Copilot get_context still answers when the auto-connection cannot be saved", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-readonly-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect-readonly"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + // The shape a read-only or permission-denied home takes from here: the + // directory the selection is written into cannot be created. + await writeFile(path.join(home.directory, "plugin-sessions"), "not a directory\n"); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + const first = await session.call( + toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.equal(first.result.isError, false); + assert.match(first.result.content[0].text, /No NeatContext Context is connected/); + assert.doesNotMatch(first.result.content[0].text, /Automatically connected/); + + // And the session is still usable afterwards, rather than hanging on every + // subsequent call. + const second = await session.call(toolCall(3, "get_context", { query: "checkout-api 5xx" })); + assert.equal(second.result.isError, false); +}); + test("Copilot get_context does not auto-connect a near-tie", async (t) => { const home = await isolatedHome("neatcontext-copilot-auto-connect-tie-"); const sessions = []; diff --git a/tests/routing-confidence.test.mjs b/tests/routing-confidence.test.mjs index 69bedf9..6e98c65 100644 --- a/tests/routing-confidence.test.mjs +++ b/tests/routing-confidence.test.mjs @@ -42,7 +42,7 @@ beforeEach(async () => { const routing = await import("../plugins/claude-code/neatcontext/src/core/routing.mjs"); const store = await import("../plugins/claude-code/neatcontext/src/core/context-store.mjs"); -const { CLOSE_RATIO, assess } = await import( +const { CLOSE_RATIO, assess, isConfidentMatch, normalizeRoutingText } = await import( "../plugins/claude-code/neatcontext/src/core/routing-candidates.mjs" ); @@ -141,6 +141,170 @@ describe("assess", () => { }); }); +describe("isConfidentMatch", () => { + // `assess` answers whether one candidate is ahead of the rest. This answers + // the other half — whether being ahead was earned — and a store of one + // context is where the two come apart: the leader there is uncontested by + // definition, so `clear` says nothing at all about the match. + const hit = (term, ...fields) => [term, fields.length > 0 ? fields : ["description"]]; + const candidate = (name, ...hits) => ({ + id: name, + name, + score: 10, + matched: hits.map(([term]) => term), + matchedFields: Object.fromEntries(hits.map(([term, fields]) => [term, fields])) + }); + + it("wants two parts of the request to agree, not two tokens", () => { + assert.equal( + isConfidentMatch(candidate("Checkout", hit("checkout"), hit("5xx")), "checkout 5xx"), + true + ); + }); + + it("counts one hyphenated word as the one word the user typed", () => { + // tokenize expands `checkout-api` to run + parts on purpose, for recall. + // Three tokens off one word is not three parts of a request agreeing. + const leader = candidate( + "Checkout", + hit("checkout-api"), + hit("checkout"), + hit("api") + ); + assert.equal(leader.matched.length, 3); + assert.equal(isConfidentMatch(leader, "checkout-api"), false); + assert.equal(isConfidentMatch(leader, "checkout-api 5xx"), false, "5xx did not match"); + }); + + it("counts a CJK run the same way, rather than as one token per character", () => { + // 订单延迟 emits its four characters and three bigrams: seven tokens, one + // request. Left uncounted, every two-character CJK question auto-connects. + const leader = candidate( + "订单系统", + hit("订"), + hit("单"), + hit("延"), + hit("迟"), + hit("订单"), + hit("单延"), + hit("延迟") + ); + assert.equal(isConfidentMatch(leader, "订单延迟"), false); + assert.equal(isConfidentMatch(leader, "订单延迟 排查"), false, "排查 did not match"); + }); + + it("does not let two filenames stand in for what a context is for", () => { + const leader = candidate("Payments", hit("deploy", "files"), hit("runbook", "files")); + assert.equal(isConfidentMatch(leader, "where is the deploy runbook?"), false); + }); + + it("counts a filename hit that also landed somewhere that means something", () => { + const leader = candidate( + "Payments", + hit("deploy", "files", "description"), + hit("runbook", "files") + ); + assert.equal(isConfidentMatch(leader, "deploy runbook rollback"), false); + assert.equal( + isConfidentMatch(candidate("Payments", hit("deploy", "files", "description"), hit("rollback")), "deploy rollback"), + true + ); + }); + + it("takes a candidate whose fields were never recorded at face value", () => { + // Older callers hand back `matched` alone. Silently scoring those at zero + // would turn a missing field map into a routing outage. + assert.equal( + isConfidentMatch({ name: "Checkout", matched: ["checkout", "5xx"] }, "checkout 5xx"), + true + ); + }); + + it("connects on the context's own name", () => { + assert.equal(isConfidentMatch(candidate("Checkout incident"), " Checkout Incident "), true); + }); + + it("connects on an alias the user wrote, when it is specific enough to be one", () => { + const leader = candidate("Windows notes", hit("windows")); + assert.equal( + isConfidentMatch(leader, "how does LM coordination work in Windows ServiceManager?", { + aliases: ["LM coordination"] + }), + true + ); + }); + + it("refuses a one-word alias found inside an ordinary sentence", () => { + // `api`, `pr`, `lm` are exactly the aliases people write, and derived cards + // generate them too. Matching one inside a sentence is weaker evidence than + // the floor it would be skipping. + const leader = candidate("Windows notes", hit("windows")); + assert.equal( + isConfidentMatch(leader, "how do I install Docker on Windows?", { aliases: ["windows"] }), + false + ); + // It is still the user's word for this context when it is the whole ask. + assert.equal(isConfidentMatch(leader, "Windows", { aliases: ["windows"] }), true); + }); + + it("refuses a punctuated one-word alias found inside an ordinary sentence", () => { + // `tokenize` splits `checkout-api` into three, so counting tokens would + // read this as a multi-word alias and let it match from inside a sentence — + // reopening the bypass for every ticket id, service name and API version + // anyone registers. One word means one word the user typed. + const leader = candidate("Checkout API", hit("checkout-api")); + for (const alias of ["checkout-api", "user_id", "api-v2", "INC-1001"]) { + assert.equal( + isConfidentMatch(leader, `why is ${alias} throwing 5xx on staging?`, { aliases: [alias] }), + false, + alias + ); + assert.equal(isConfidentMatch(leader, alias, { aliases: [alias] }), true, alias); + } + }); + + it("refuses a multi-word alias that tokenizes down to one word", () => { + // `the api` is two words, but `tokenize` drops the stopword and leaves one, + // and a one-token contiguous check is just "does this word appear anywhere". + // `the API`, `our PR`, `how LM works` are how people write these down. + const leader = candidate("Payments API", hit("api")); + for (const [alias, query] of [ + ["the api", "why is the api throwing 5xx today?"], + ["our api", "why is the api throwing 5xx today?"], + ["how api", "why is the api throwing 5xx today?"], + ["our pr", "why was our pr merged so fast today?"] + ]) { + assert.equal(isConfidentMatch(leader, query, { aliases: [alias] }), false, alias); + } + // Two words that both survive still route from inside a sentence. + assert.equal( + isConfidentMatch(leader, "why is the payments api throwing 5xx today?", { + aliases: ["payments api"] + }), + true + ); + }); + + it("ignores an alias with nothing matchable in it", () => { + const leader = candidate("Notes", hit("notes")); + assert.equal(isConfidentMatch(leader, "a note about x", { aliases: ["!!", "-"] }), false); + }); + + it("refuses when there is no request to match against", () => { + const leader = candidate("Checkout", hit("checkout"), hit("5xx")); + assert.equal(isConfidentMatch(leader, " "), false); + assert.equal(isConfidentMatch(leader, undefined), false); + }); + + it("refuses a candidate that matched nothing", () => { + assert.equal(isConfidentMatch({ name: "Checkout" }, "checkout 5xx"), false); + }); + + it("normalizes the way the rest of routing does", () => { + assert.equal(normalizeRoutingText(" Checkout API "), "checkout api"); + }); +}); + describe("the default mode", () => { it("is auto, because asking is now the route's decision and not the dial's", async () => { // Auto only became safe once a near-tie asks on its own. Before that, "ask @@ -193,6 +357,30 @@ describe("renderShortlist with a decision", () => { }); }); +describe("renderMenu with a decision", () => { + // The full menu is what goes out below SHORTLIST_MIN_CONTEXTS, which is where + // most machines live. A tie the plugin refused to act on has to reach the + // model here too — otherwise the plugin declines, explains nothing, and the + // model picks one of the two anyway. + const entries = [ + { id: "a", name: "Codex plugin", useWhen: "packaging", aliases: [] }, + { id: "b", name: "Kimi plugin", useWhen: "packaging", aliases: [] } + ]; + const scored = entries.map((entry, index) => ({ ...entry, score: index === 0 ? 10 : 9.8 })); + + it("carries the near-tie into the full menu", () => { + const text = routing.renderMenu(entries, { mode: "auto", decision: assess(scored) }); + assert.match(text, /\*\*Codex plugin\*\* and \*\*Kimi plugin\*\* match the request about equally well/); + assert.match(text, /in auto mode too/); + }); + + it("says nothing about ties when there is a clear leader, or no decision", () => { + const clear = assess([{ ...entries[0], score: 10 }, { ...entries[1], score: 2 }]); + assert.ok(!routing.renderMenu(entries, { mode: "auto", decision: clear }).includes("equally well")); + assert.ok(!routing.renderMenu(entries, { mode: "auto" }).includes("equally well")); + }); +}); + describe("the bridge decides from real scores", () => { // Two contexts described identically: nothing but the name separates them, // so a request about what they share is a genuine coin flip. diff --git a/tests/routing-unconnected.test.mjs b/tests/routing-unconnected.test.mjs index 284b812..e31a0a0 100644 --- a/tests/routing-unconnected.test.mjs +++ b/tests/routing-unconnected.test.mjs @@ -184,6 +184,41 @@ describe("get_context with nothing connected", () => { } }); + // Auto-connect firing first does not retire either fault this suite was + // written for — it only moves them off the path the test above walks. Both + // are still reachable on every call that declines to connect, which is most + // of them, so each keeps a home that still runs. + + it("never answers 'what now?' with a slash command when it declines to connect", async () => { + // Two contexts describing the same thing: the request matches both, so + // auto-connect correctly refuses and the text below is what the model gets. + await create("Codex packaging", "plugin packaging manifests and marketplace steps"); + await create("Kimi packaging", "plugin packaging manifests and marketplace steps"); + const session = bridge("declines-cleanly"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + const text = await ask(session, "plugin packaging manifests marketplace steps"); + + assert.doesNotMatch(text, /Automatically connected/); + assert.match(text, /connect a clear choice with `use_context`/); + assert.match(text, /do not ask the user to run a command/i); + // The regression itself: the old text opened by telling the model to send + // the user to /neatcontext:use, and that is what it acted on. + assert.doesNotMatch( + text.split("## Contexts")[0], + /\/neatcontext:use/, + "the lead paragraph must not answer 'what now?' with a slash command" + ); + // And having refused a near-tie itself, it has to say so. Silently + // declining leaves the model to pick one of the two at random, which is + // the wrong-context re-grounding the tie check exists to prevent. + assert.match(text, /match the request about equally well/); + assert.match(text, /\*\*Codex packaging\*\* and \*\*Kimi packaging\*\*/); + } finally { + await session.close(); + } + }); + it("is reachable on a machine upgraded from a pre-#77 routing file", async () => { await create("Incident", "checkout-api 5xx from pgbouncer pool exhaustion"); const state = await readRoutingFile(); @@ -195,6 +230,35 @@ describe("get_context with nothing connected", () => { const text = await ask(session, "checkout-api 5xx"); assert.match(text, /Automatically connected "Incident"/); assert.match(text, /connected context: Incident/i); + assert.match(text, /Routing is on \(auto\)/); + } finally { + await session.close(); + } + }); + + it("lets the session switch unprompted on a migrated file", async () => { + await create("Incident", "checkout-api 5xx from pgbouncer pool exhaustion"); + const state = await readRoutingFile(); + await writeRoutingFile({ ...state, schema: 1, mode: "ask" }); + + const session = bridge("upgraded-machine-unprompted"); + try { + await session.send("initialize", { protocolVersion: "2025-11-25" }); + // Nothing here matches, so nothing is connected for the model — which is + // the state the baked-in "ask" used to trap a session in. + const text = await ask(session, "what is the capital of France?"); + assert.doesNotMatch(text, /Automatically connected/); + assert.match(text, /Routing is on \(auto\)/); + + // And the switch it was told to make actually goes through, unprompted, + // which is what the baked-in ask was refusing. This is the end-to-end + // proof through switchPolicy that auto-connect's own path never walks. + const used = await session.send("tools/call", { + name: "use_context", + arguments: { context: "Incident", reason: "checkout 5xx" } + }); + assert.equal(used.result.isError, false); + assert.match(used.result.content[0].text, /Switched this session to "Incident"/); } finally { await session.close(); } From 8f63e5de76bf706c0092f2cdba9a064ff0c90d8b Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Tue, 11 Aug 2026 17:11:16 -0700 Subject: [PATCH 4/5] Round two of maintainer review on the auto-connect path Fourteen findings, all of them fair. The two that mattered most were the same bug seen from two sides: a single compound word carried enough tokens to clear the agreement floor on its own, and a run of Chinese carried far more. Counting distinct carried tokens does not fix it -- checkout-api carries three by itself. Agreement has to be independent on both sides, so agreeingTerms now computes a maximum matching between the parts of the request and the things they agreed on. Exact rather than greedy, because greedy is order-dependent and rewording a sentence must not change where it routes. A no-space script is one part of the request however long it is, so it can supply at most one pairing and can never reach the floor. That is a real limitation and it is documented and tested rather than papered over: Chinese and kanji-dense Japanese need an exact name match or a whole-request alias to connect unasked. Korean is written with spaces and is unaffected. The decision log could be emptied of the user's own routes by the machine's: automatic entries now cap separately at 20 and merge back in time order, so familiarity still sees what the user actually chose. On the bridge, the routing pass is now the single source for the whole call. Ranking happens once and the shortlist slices it, ties are assessed whether or not auto-connect was ever eligible, a stale selection reads as nothing connected, the connection is recorded before the log that describes it, a cross-session refusal disqualifies rather than discounts, sessionId() is inside the guard that keeps a dead working directory from silencing every answer, and an auto-connection stops the previous context's extensions the way an explicit switch does. Every fix above is pinned by a test that fails without it. --- .../src/core/routing-candidates.mjs | 98 +++++-- .../plugins/neatcontext/src/core/routing.mjs | 33 ++- .../src/core/routing-candidates.mjs | 98 +++++-- .../neatcontext/src/core/routing.mjs | 33 ++- .../neatcontext/src/copilot/mcp-bridge.mjs | 256 ++++++++++-------- .../src/core/routing-candidates.mjs | 98 +++++-- .../copilot/neatcontext/src/core/routing.mjs | 33 ++- .../src/core/routing-candidates.mjs | 98 +++++-- .../neatcontext/src/core/routing.mjs | 33 ++- .../src/core/routing-candidates.mjs | 98 +++++-- plugins/pi/neatcontext/src/core/routing.mjs | 33 ++- shared/core/routing-candidates.mjs | 98 +++++-- shared/core/routing.mjs | 33 ++- tests/copilot-plugin.test.mjs | 231 +++++++++++++++- tests/fake-extension-server.mjs | 7 + tests/routing-confidence.test.mjs | 91 +++++++ tests/routing.test.mjs | 47 ++++ 17 files changed, 1146 insertions(+), 272 deletions(-) diff --git a/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs b/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs index ce0eeb8..a6455c8 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs @@ -125,18 +125,49 @@ export function normalizeRoutingText(text) { return text.trim().toLowerCase().replace(/\s+/g, " "); } -// The request split the way the user wrote it, deduplicated: one entry per -// whitespace-separated run. -// -// This is the unit the floor counts, and it has to be, because a token is not -// one. `tokenize` deliberately expands a single `checkout-api` into -// `[checkout-api, checkout, api]` and a two-character CJK request into seven -// tokens — good for recall, but counting those as agreement means one word, or -// any CJK request at all, clears a floor meant to require two. +// The request as the user wrote it: one entry per whitespace-separated run. +// +// This is the unit the floor counts parts of the request in, and both the term +// floor and the alias floor have to count it the same way, so the split lives +// in one place rather than being repeated as a convention across six synced +// copies. +function words(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean); +} + +// Deduplicated, because the same word typed twice is not two parts agreeing. function queryTerms(query) { - return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); + return new Set(words(query)); } +// How much of the request genuinely agreed with this context. +// +// Evidence has to be independent on *both* sides, and each side alone is a +// bypass the other does not catch: +// +// - Counting parts of the request lets one concept spelled two ways pass as +// two. "what does user_id mean for a user?" is two words agreeing on the +// single token `user`. +// - Counting things agreed on lets one compound word pass as several. +// `tokenize` expands `checkout-api` into `[checkout-api, checkout, api]`, +// so a request of that one word would otherwise clear a floor of three. +// +// So what is counted is pairings: the largest set of (part of the request, +// thing it agreed on) pairs where no two pairs share either side. That is a +// maximum bipartite matching, and it is worth being exact rather than greedy +// about it — a greedy pass gives different answers for the same words in a +// different order, and "why does rewording the sentence change the route?" is +// not a question this should ever raise. +// +// One consequence is deliberate and documented: a script written without +// spaces — Chinese, or kanji-dense Japanese — is a single part of the request +// however long it is, so it can contribute at most one pairing and can never +// clear the floor. Auto-connect is therefore unreachable for those requests +// without an exact name or whole-request alias match, until this can segment +// them. That is a real limitation rather than a rounding error, and it is the +// conservative direction: the routing menu still answers, exactly as it does +// today for every user. It is narrower than "CJK" — Korean is written with +// spaces between eojeol and goes through the ordinary path. function agreeingTerms(candidate, query) { const carried = new Set( (candidate.matched ?? []).filter((term) => { @@ -147,13 +178,38 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } - let agreeing = 0; + const options = []; for (const term of queryTerms(query)) { - if (tokenize(term).some((token) => carried.has(token))) { - agreeing += 1; + const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); + if (agreed.length > 0) { + options.push(agreed); } } - return agreeing; + + // Kuhn's algorithm: give each part of the request something to agree on, + // letting an earlier one give up its choice whenever it has another. + const takenBy = new Map(); + const claim = (part, tried) => { + for (const token of options[part]) { + if (tried.has(token)) { + continue; + } + tried.add(token); + const holder = takenBy.get(token); + if (holder === undefined || claim(holder, tried)) { + takenBy.set(token, part); + return true; + } + } + return false; + }; + let paired = 0; + for (let part = 0; part < options.length; part += 1) { + if (claim(part, new Set())) { + paired += 1; + } + } + return paired; } // An alias is the one routing signal the user authored by hand, at the moment @@ -164,21 +220,17 @@ function agreeingTerms(candidate, query) { // alias therefore has to be the whole request; a longer one has to appear // contiguously in the request's tokens. // -// One word means one word the user typed, counted the way `queryTerms` counts -// the request — not the tokens the index derived from it. `tokenize` expands -// `checkout-api` into three and `user_id` into three, and reading that as a -// multi-word alias would reopen the bypass for every ticket id, service name -// and API version anyone is likely to register. +// One word means one word the user typed, counted by `words` exactly as the +// term floor counts the request — not the tokens the index derived from it. +// `tokenize` expands `checkout-api` into three and `user_id` into three, and +// reading that as a multi-word alias would reopen the bypass for every ticket +// id, service name and API version anyone is likely to register. // // It has to survive tokenizing as two, as well. `the api` is two words the user // typed, but `tokenize` drops the stopword and leaves one, and a one-token // contiguous check is just "does this word appear anywhere" — the very test the // first floor exists to prevent. `the API`, `our PR`, `how LM works` are how // people write these aliases down, so both floors have to hold. -function wordCount(text) { - return normalizeRoutingText(text).split(" ").filter(Boolean).length; -} - function matchesAlias(aliases, query) { const normalized = normalizeRoutingText(query); const queryTokens = tokenize(query); @@ -187,7 +239,7 @@ function matchesAlias(aliases, query) { if (aliasTokens.length === 0) { return false; } - if (wordCount(alias) < 2 || aliasTokens.length < 2) { + if (words(alias).length < 2 || aliasTokens.length < 2) { return normalizeRoutingText(alias) === normalized; } return queryTokens.some((_, start) => diff --git a/codex-marketplace/plugins/neatcontext/src/core/routing.mjs b/codex-marketplace/plugins/neatcontext/src/core/routing.mjs index a0a401c..266e040 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/routing.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/routing.mjs @@ -47,6 +47,16 @@ const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; + +// Automatic routes are capped separately, and far shorter. +// +// They arrive at roughly one per new session, so a single shared cap would see +// them evict the manual selections this log exists for — and `familiarity` +// skipping them is exactly what would keep anyone from noticing the ground +// truth had drained away. Manual decisions keep their own hundred; the machine +// ones keep a short tail, which is all that is ever read back when working out +// why a session routed the way it did. +const MAX_AUTOMATIC_DECISIONS = 20; const MAX_SESSIONS = 20; // How long a refusal keeps counting for. @@ -192,7 +202,7 @@ async function writeRouting(state) { ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), - decisions: state.decisions.slice(-MAX_DECISIONS) + decisions: capDecisions(state.decisions) }, null, 2 @@ -201,6 +211,20 @@ async function writeRouting(state) { ); } +// Trimmed in two buckets rather than one, so the machine's own routes cannot +// push the user's out of the record. Merged back in time order afterwards, +// because everything downstream reads this as a chronology. +function capDecisions(decisions) { + const manual = []; + const automatic = []; + for (const decision of decisions) { + (decision?.automatic === true ? automatic : manual).push(decision); + } + return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( + (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) + ); +} + async function update(mutate) { const state = await readRouting(); const result = mutate(state); @@ -482,7 +506,7 @@ export function renderMenu(entries, { connectedId, mode, decision } = {}) { lines.push(""); const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push(tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); @@ -547,9 +571,12 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); + // Its own paragraph: it is the one line asking the model to stop and ask, + // and run together with the instructions around it that is what it stops + // looking like. const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push("", tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); diff --git a/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs b/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs index ce0eeb8..a6455c8 100644 --- a/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs @@ -125,18 +125,49 @@ export function normalizeRoutingText(text) { return text.trim().toLowerCase().replace(/\s+/g, " "); } -// The request split the way the user wrote it, deduplicated: one entry per -// whitespace-separated run. -// -// This is the unit the floor counts, and it has to be, because a token is not -// one. `tokenize` deliberately expands a single `checkout-api` into -// `[checkout-api, checkout, api]` and a two-character CJK request into seven -// tokens — good for recall, but counting those as agreement means one word, or -// any CJK request at all, clears a floor meant to require two. +// The request as the user wrote it: one entry per whitespace-separated run. +// +// This is the unit the floor counts parts of the request in, and both the term +// floor and the alias floor have to count it the same way, so the split lives +// in one place rather than being repeated as a convention across six synced +// copies. +function words(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean); +} + +// Deduplicated, because the same word typed twice is not two parts agreeing. function queryTerms(query) { - return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); + return new Set(words(query)); } +// How much of the request genuinely agreed with this context. +// +// Evidence has to be independent on *both* sides, and each side alone is a +// bypass the other does not catch: +// +// - Counting parts of the request lets one concept spelled two ways pass as +// two. "what does user_id mean for a user?" is two words agreeing on the +// single token `user`. +// - Counting things agreed on lets one compound word pass as several. +// `tokenize` expands `checkout-api` into `[checkout-api, checkout, api]`, +// so a request of that one word would otherwise clear a floor of three. +// +// So what is counted is pairings: the largest set of (part of the request, +// thing it agreed on) pairs where no two pairs share either side. That is a +// maximum bipartite matching, and it is worth being exact rather than greedy +// about it — a greedy pass gives different answers for the same words in a +// different order, and "why does rewording the sentence change the route?" is +// not a question this should ever raise. +// +// One consequence is deliberate and documented: a script written without +// spaces — Chinese, or kanji-dense Japanese — is a single part of the request +// however long it is, so it can contribute at most one pairing and can never +// clear the floor. Auto-connect is therefore unreachable for those requests +// without an exact name or whole-request alias match, until this can segment +// them. That is a real limitation rather than a rounding error, and it is the +// conservative direction: the routing menu still answers, exactly as it does +// today for every user. It is narrower than "CJK" — Korean is written with +// spaces between eojeol and goes through the ordinary path. function agreeingTerms(candidate, query) { const carried = new Set( (candidate.matched ?? []).filter((term) => { @@ -147,13 +178,38 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } - let agreeing = 0; + const options = []; for (const term of queryTerms(query)) { - if (tokenize(term).some((token) => carried.has(token))) { - agreeing += 1; + const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); + if (agreed.length > 0) { + options.push(agreed); } } - return agreeing; + + // Kuhn's algorithm: give each part of the request something to agree on, + // letting an earlier one give up its choice whenever it has another. + const takenBy = new Map(); + const claim = (part, tried) => { + for (const token of options[part]) { + if (tried.has(token)) { + continue; + } + tried.add(token); + const holder = takenBy.get(token); + if (holder === undefined || claim(holder, tried)) { + takenBy.set(token, part); + return true; + } + } + return false; + }; + let paired = 0; + for (let part = 0; part < options.length; part += 1) { + if (claim(part, new Set())) { + paired += 1; + } + } + return paired; } // An alias is the one routing signal the user authored by hand, at the moment @@ -164,21 +220,17 @@ function agreeingTerms(candidate, query) { // alias therefore has to be the whole request; a longer one has to appear // contiguously in the request's tokens. // -// One word means one word the user typed, counted the way `queryTerms` counts -// the request — not the tokens the index derived from it. `tokenize` expands -// `checkout-api` into three and `user_id` into three, and reading that as a -// multi-word alias would reopen the bypass for every ticket id, service name -// and API version anyone is likely to register. +// One word means one word the user typed, counted by `words` exactly as the +// term floor counts the request — not the tokens the index derived from it. +// `tokenize` expands `checkout-api` into three and `user_id` into three, and +// reading that as a multi-word alias would reopen the bypass for every ticket +// id, service name and API version anyone is likely to register. // // It has to survive tokenizing as two, as well. `the api` is two words the user // typed, but `tokenize` drops the stopword and leaves one, and a one-token // contiguous check is just "does this word appear anywhere" — the very test the // first floor exists to prevent. `the API`, `our PR`, `how LM works` are how // people write these aliases down, so both floors have to hold. -function wordCount(text) { - return normalizeRoutingText(text).split(" ").filter(Boolean).length; -} - function matchesAlias(aliases, query) { const normalized = normalizeRoutingText(query); const queryTokens = tokenize(query); @@ -187,7 +239,7 @@ function matchesAlias(aliases, query) { if (aliasTokens.length === 0) { return false; } - if (wordCount(alias) < 2 || aliasTokens.length < 2) { + if (words(alias).length < 2 || aliasTokens.length < 2) { return normalizeRoutingText(alias) === normalized; } return queryTokens.some((_, start) => diff --git a/plugins/claude-code/neatcontext/src/core/routing.mjs b/plugins/claude-code/neatcontext/src/core/routing.mjs index a0a401c..266e040 100644 --- a/plugins/claude-code/neatcontext/src/core/routing.mjs +++ b/plugins/claude-code/neatcontext/src/core/routing.mjs @@ -47,6 +47,16 @@ const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; + +// Automatic routes are capped separately, and far shorter. +// +// They arrive at roughly one per new session, so a single shared cap would see +// them evict the manual selections this log exists for — and `familiarity` +// skipping them is exactly what would keep anyone from noticing the ground +// truth had drained away. Manual decisions keep their own hundred; the machine +// ones keep a short tail, which is all that is ever read back when working out +// why a session routed the way it did. +const MAX_AUTOMATIC_DECISIONS = 20; const MAX_SESSIONS = 20; // How long a refusal keeps counting for. @@ -192,7 +202,7 @@ async function writeRouting(state) { ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), - decisions: state.decisions.slice(-MAX_DECISIONS) + decisions: capDecisions(state.decisions) }, null, 2 @@ -201,6 +211,20 @@ async function writeRouting(state) { ); } +// Trimmed in two buckets rather than one, so the machine's own routes cannot +// push the user's out of the record. Merged back in time order afterwards, +// because everything downstream reads this as a chronology. +function capDecisions(decisions) { + const manual = []; + const automatic = []; + for (const decision of decisions) { + (decision?.automatic === true ? automatic : manual).push(decision); + } + return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( + (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) + ); +} + async function update(mutate) { const state = await readRouting(); const result = mutate(state); @@ -482,7 +506,7 @@ export function renderMenu(entries, { connectedId, mode, decision } = {}) { lines.push(""); const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push(tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); @@ -547,9 +571,12 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); + // Its own paragraph: it is the one line asking the model to stop and ask, + // and run together with the instructions around it that is what it stops + // looking like. const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push("", tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); diff --git a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs index adec3d5..6314d1a 100644 --- a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs +++ b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs @@ -16,13 +16,14 @@ import { CONTEXT_MISSING_MESSAGE, listKnowledgeFiles, listContexts, - readContext, renderContext } from "../core/context-store.mjs"; import { createExtensionHost, renderExtensionStatus } from "../core/extension-runtime.mjs"; import { parseQualifiedToolName } from "../core/extensions.mjs"; import { addAlias, + DEFAULT_MODE, + declineFactor, menuEntries, noteDecision, noteDeclined, @@ -242,42 +243,34 @@ function jsonRpcResult(id, result) { // whether a menu is about to follow this text, and therefore whether pointing // at a slash command is the honest answer or the one that breaks routing. // -// The routing pass this call already made is passed in rather than re-read, -// and it is also what says whether automatic matching ran at all. -async function nothingConnectedText(pass) { - const contexts = pass?.contexts ?? (await listContexts().catch(() => [])); - if (contexts.length === 0) { +// The routing pass this call already made is what this is built from, and it +// is also what says whether automatic matching ran at all. +function nothingConnectedText(pass) { + if (pass.contexts.length === 0) { return NOTHING_EXISTS; } - const state = pass?.state ?? (await readRouting().catch(() => ({ sessions: {} }))); - const mode = resolveMode(state, sessionId()); - if (mode === "manual") { + if (pass.mode === "manual") { return NOTHING_CONNECTED; } - return mode === "ask" ? NOTHING_CONNECTED_ASK : nothingConnectedRoutable(pass?.assessed === true); + return pass.mode === "ask" ? NOTHING_CONNECTED_ASK : nothingConnectedRoutable(pass.assessed); } // The selected context, or null when nothing is selected. A selection // whose context was deleted out-of-band resolves to `missing` so get_context // can say what happened. // -// A context this call just connected is passed straight through: it was read -// out of the same listing a moment ago, and re-reading the selection file only -// to look it up again would be two disk hits to learn what is already in hand. -// The same listing answers the ordinary case, so a call that made one pass over -// the store makes exactly one. -async function activeContext(pass) { - if (pass?.connected) { +// Answered entirely from the pass: the listing it holds was read a moment ago, +// and `readContext` is itself a lookup in that same listing. One resolution +// path rather than two, so there is nowhere for the two to drift apart. +function activeContext(pass) { + if (pass.connected) { return { record: pass.connected }; } - const selection = pass ? pass.selection : await readSelection().catch(() => null); - if (!selection || selection.available === false) { + if (!pass.connectedId) { return null; } - const record = pass - ? (pass.contexts.find((context) => context.id === selection.contextId) ?? null) - : await readContext(selection.contextId).catch(() => null); - return record ? { record } : { missing: true, name: selection.contextName }; + const record = pass.contexts.find((context) => context.id === pass.connectedId) ?? null; + return record ? { record } : { missing: true, name: pass.selection?.contextName }; } // --- Extensions: what the connected context can reach -------------------------- @@ -305,7 +298,7 @@ function dependsOnExtensions(message) { ); } -async function contextResponse(message, context, pass = null) { +async function contextResponse(message, context, pass) { const { id, method, params } = message; if (id === undefined || id === null) { return null; // notification: nothing to answer @@ -330,7 +323,7 @@ async function contextResponse(message, context, pass = null) { if (method === "tools/call" && params?.name === GET_CONTEXT_TOOL.name) { if (!context) { return jsonRpcResult(id, { - content: [{ type: "text", text: await nothingConnectedText(pass) }], + content: [{ type: "text", text: nothingConnectedText(pass) }], isError: false }); } @@ -380,25 +373,18 @@ const rankContexts = createRoutingIndex({ (await listKnowledgeFiles(record.knowledgeFolder, { limit: 60 })).files }); -async function routingMenu(query, pass) { - const [contexts, state] = pass - ? [pass.contexts, pass.state] - : await Promise.all([listContexts(), readRouting()]); - const connectedId = pass - ? (pass.connected?.id ?? pass.selection?.contextId ?? null) - : ((await readSelection().catch(() => null))?.contextId ?? null); - const options = { connectedId, mode: resolveMode(state, sessionId()) }; - const entries = menuEntries(contexts, state); - const shortlist = await shortlistFor(contexts, state, entries, query, connectedId); - if (shortlist) { - return renderShortlist(shortlist, { ...options, decision: assess(shortlist) }); - } - // The full menu carries the tie the pass found, if it found one. A store - // below SHORTLIST_MIN_CONTEXTS never builds a shortlist, so this is the only - // way a near-tie the plugin already refused to act on reaches the model at - // all — and without it the plugin declines, says nothing about why, and the - // model picks one of the two anyway. - return renderMenu(entries, { ...options, decision: pass?.decision }); +function routingMenu(pass) { + const options = { connectedId: pass.connectedId, mode: pass.mode }; + const entries = menuEntries(pass.contexts, pass.state); + const shortlist = shortlistFor(entries, pass.ranked); + // The same verdict either way. A near-tie is a fact about the ranking, not + // about how much of it is being shown, and a store below + // SHORTLIST_MIN_CONTEXTS never builds a shortlist at all — so reading it off + // the shortlist alone left the full menu, the one place the model has least + // to go on, as the only caller that never heard about it. + return shortlist + ? renderShortlist(shortlist, { ...options, decision: pass.decision }) + : renderMenu(entries, { ...options, decision: pass.decision }); } // `get_context` is already the session asking the plugin to route this request. @@ -413,61 +399,83 @@ async function routingMenu(query, pass) { // made every `get_context` with a query hit the disk three times over for the // same answer. // +// Built for every message, not only the ones that can auto-connect, so there is +// one way to resolve a context rather than a pass-shaped path and a re-reading +// path that have to be kept agreeing with each other. Without a query it stops +// after the reads, which is exactly what the old path did. +// +// Ranking happens whenever there is a request to rank, before any question of +// whether *this* call may act on it: a near-tie is a property of the ranking, +// and the menu needs to say so even when auto-connect was never on the table. +// // The confidence rule itself lives in core beside `assess`, not here. Copilot // is the first host to act on it and for now the only one; the other four share // this machine's `~/.neatcontext` and keep the old behavior until they are // wired up too, which is a staged rollout rather than a permanent split. async function routingPass(query) { const asked = typeof query === "string" && query.trim().length > 0; - const id = sessionId(); - const selection = await readSelection().catch(() => null); - const [contexts, state] = await Promise.all([ - listContexts().catch(() => []), - readRouting().catch(() => ({ sessions: {}, cards: {} })) - ]); - const pass = { contexts, state, selection, connected: null, decision: null, assessed: false }; - - // Everything that stops the pass before it ranks anything, in one place, so - // `assessed` stays a statement about what actually happened. - // - // A selection carrying a `contextId` covers both "already connected" and - // "pointed at a context that is gone, and `readSelection` just cleared the - // file". The second is deliberate: a user whose context vanished should be - // told that, not silently re-grounded in a different one on the next answer. - if ( - !asked || - selection?.contextId || - contexts.length === 0 || - resolveMode(state, id) !== "auto" || - // Without a session id published by the host, one selection file is shared - // by every window open on this workspace. A model or a user calling - // `use_context` at least announces the switch; a keyword hit in one window - // would silently re-ground the conversation in the next. - !hasHostSessionId() - ) { - return pass; - } + const pass = { + contexts: [], + state: { sessions: {}, cards: {} }, + selection: null, + connectedId: null, + mode: DEFAULT_MODE, + ranked: null, + decision: null, + connected: null, + assessed: false + }; - // Every candidate, not a top slice: the tie check is only as good as the - // field it can see. - // - // Everything from here is inside one guard. An auto-connection that cannot be - // made is a missed optimization, and that is all it may ever cost — unguarded, - // a home this process cannot write to turned every `get_context` in the - // session into a request that is never answered at all: the write rejected, - // `main` swallowed it, and nothing was written to stdout. + // Everything is inside one guard, starting with `sessionId()` — it reads + // `process.cwd()`, which throws outright once the working directory has been + // removed under a long-lived server. An auto-connection that cannot be made + // is a missed optimization, and that is all any of this may ever cost: + // unguarded, one rejection here turned every `get_context` in the session + // into a request that is never answered at all, because `main` swallowed it + // and nothing was written to stdout. try { - const ranked = await rankContexts(contexts, state, query, { + const id = sessionId(); + const selection = await readSelection().catch(() => null); + const [contexts, state] = await Promise.all([ + listContexts().catch(() => []), + readRouting().catch(() => ({ sessions: {}, cards: {} })) + ]); + pass.selection = selection; + pass.contexts = contexts; + pass.state = state; + pass.mode = resolveMode(state, id); + // A selection whose context is gone is nothing connected — `readSelection` + // has just deleted the file — and everything downstream has to agree on + // that. Read as connected, one response says "no Context is connected" + // while carrying the guards written for a session that has somewhere to + // leave, which is the suppression this whole path exists to remove. + pass.connectedId = selection?.available === false ? null : (selection?.contextId ?? null); + + if (!asked || contexts.length === 0) { + return pass; + } + + // Every candidate, not a top slice: the tie check is only as good as the + // field it can see. The shortlist takes its own slice of this afterwards + // rather than ranking the corpus a second time. + pass.ranked = await rankContexts(contexts, state, query, { limit: contexts.length, - connectedId: null + connectedId: pass.connectedId }); - pass.decision = assess(ranked); + pass.decision = assess(pass.ranked); + + // From here it is about acting unasked. `assessed` stays a statement about + // that specific question, so the nothing-connected text only claims a match + // was looked for when one actually was. + if (pass.connectedId || pass.mode !== "auto" || !hasHostSessionId()) { + return pass; + } pass.assessed = true; if (pass.decision.verdict !== "clear") { return pass; } - const leader = ranked[0]; + const leader = pass.ranked[0]; const target = contexts.find((context) => context.id === leader.id); if (!target || !isConfidentMatch(leader, query, { aliases: aliasesOf(state, target.id) })) { return pass; @@ -480,7 +488,25 @@ async function routingPass(query) { return pass; } + // A refusal the user made in another session only discounts the score, and + // a discount cannot change an outcome the leader was going to win anyway — + // which in a small store is every outcome. That was survivable while this + // route went through the model, because calling `use_context` announces the + // switch and gives the user somewhere to say no again. Acting unasked + // removes the announcement, so a live refusal disqualifies it outright. + if (declineFactor(state, target.id) < 1) { + return pass; + } + await applySelection(target); + // Recorded before the decision log is written, because the selection file + // is now pointing at the target whatever happens next. Left until after, + // a `noteDecision` failure produced a pass that said nothing was connected + // and a disk that said otherwise — and the menu that followed offered the + // context the session was already on, which `use_context` then refused as + // "already connected. Nothing to switch." + pass.connected = target; + pass.connectedId = target.id; await noteDecision({ sessionId: id, from: null, @@ -490,7 +516,6 @@ async function routingPass(query) { requested: false, automatic: true }); - pass.connected = target; } catch { return pass; } @@ -505,25 +530,18 @@ function aliasesOf(state, contextId) { // that narrowing gains anything, and at least one that actually matched. Any of // them missing and the full menu goes out instead — a session is never left // with less to work with than it has today. -async function shortlistFor(contexts, state, entries, query, connectedId) { - if ( - typeof query !== "string" || - query.trim().length === 0 || - entries.length < SHORTLIST_MIN_CONTEXTS - ) { - return null; - } - const ranked = await rankContexts(contexts, state, query, { - limit: SHORTLIST_LIMIT, - connectedId - }); - if (ranked.length === 0) { +// A slice of the ranking the pass already made, not a second one. Ranking the +// corpus twice per call was the larger half of the work: BM25 over every +// document, then a decline lookup and a walk of the decision log per candidate, +// all to arrive at a prefix of a list that was already in hand. +function shortlistFor(entries, ranked) { + if (!ranked || ranked.length === 0 || entries.length < SHORTLIST_MIN_CONTEXTS) { return null; } const byId = new Map(entries.map((entry) => [entry.id, entry])); // The score travels with the entry because how far ahead the leader is // decides whether the shortlist names a winner or asks a question. - return ranked.map((result) => ({ + return ranked.slice(0, SHORTLIST_LIMIT).map((result) => ({ ...byId.get(result.id), matched: result.matched, score: result.score @@ -648,15 +666,15 @@ let lastVersion = undefined; // change this; so does the routing mode, because leaving manual has to make the // routing tools appear without waiting for a restart. async function currentVersion() { - const mode = resolveMode(await readRouting(), sessionId()); - const context = await activeContext(); + const pass = await routingPass(); + const context = activeContext(pass); // The extension signature is read from what the last resolve found, never by // starting anything: this runs on a timer, and a poll must not spawn a server. const extensions = extensionHost.signature(context?.record ?? null); if (context) { - return `${mode}/${context.missing ? "context:missing" : context.record.id}/${extensions}`; + return `${pass.mode}/${context.missing ? "context:missing" : context.record.id}/${extensions}`; } - return `${mode}/none/${extensions}`; + return `${pass.mode}/none/${extensions}`; } async function handleMessage(message) { @@ -671,15 +689,23 @@ async function handleMessage(message) { const isGetContext = message.method === "tools/call" && message.params?.name === GET_CONTEXT_TOOL.name; - const pass = isGetContext ? await routingPass(message.params?.arguments?.query) : null; - const context = await activeContext(pass); + const pass = await routingPass(isGetContext ? message.params?.arguments?.query : undefined); + const context = activeContext(pass); // Extensions are deliberately not resolved on the turn that auto-connected. // Resolving one starts the user's own server, and this is the one connection // nobody asked for out loud: the announcement goes out first, and anything // bound to the context starts on the next call that actually needs it. - if (dependsOnExtensions(message) && !pass?.connected) { + // + // Deferring the start is not a reason to defer the teardown, though. Dropping + // the previous context's live clients is the other half of `resolve`, and + // `extension-runtime` states the invariant absolutely: nothing the previous + // context started stays reachable from this one. Clearing only the tool and + // status lists here left the host holding connections a qualified tool name + // could still be proxied to. + if (dependsOnExtensions(message) && !pass.connected) { await refreshExtensions(context); - } else if (pass?.connected) { + } else if (pass.connected) { + extensionHost.dispose(); extensionTools = []; extensionStatuses = []; } @@ -715,13 +741,13 @@ async function handleMessage(message) { // // The connection rule goes last, so it is the closest thing to the answer the // session is about to write — and it is the one part that is never omitted. -async function pluginNotes(query, pass) { - const menu = await routingMenu(query, pass); +async function pluginNotes(pass) { + const menu = routingMenu(pass); return menu ? `${menu}\n\n${CONNECTION_RULE}` : CONNECTION_RULE; } -async function withNotes(response, place, query, pass) { - const notes = await pluginNotes(query, pass); +async function withNotes(response, place, pass) { + const notes = await pluginNotes(pass); if (place === "instructions") { const existing = response.result.instructions; return { @@ -765,9 +791,9 @@ function prependAutoConnection(response, contextName) { }; } -async function shapeResponse(message, response, pass = null) { +async function shapeResponse(message, response, pass) { if (message.method === "initialize" && response.result) { - return withNotes(response, "instructions"); + return withNotes(response, "instructions", pass); } if (message.method === "tools/list") { return await withRoutingTools(response); @@ -776,8 +802,8 @@ async function shapeResponse(message, response, pass = null) { // The handshake has no request to match against, so only this path can // narrow the menu — which is also the path that is re-read every turn. return prependAutoConnection( - await withNotes(response, "content", message.params?.arguments?.query, pass), - pass?.connected?.name ?? null + await withNotes(response, "content", pass), + pass.connected?.name ?? null ); } return response; diff --git a/plugins/copilot/neatcontext/src/core/routing-candidates.mjs b/plugins/copilot/neatcontext/src/core/routing-candidates.mjs index ce0eeb8..a6455c8 100644 --- a/plugins/copilot/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/copilot/neatcontext/src/core/routing-candidates.mjs @@ -125,18 +125,49 @@ export function normalizeRoutingText(text) { return text.trim().toLowerCase().replace(/\s+/g, " "); } -// The request split the way the user wrote it, deduplicated: one entry per -// whitespace-separated run. -// -// This is the unit the floor counts, and it has to be, because a token is not -// one. `tokenize` deliberately expands a single `checkout-api` into -// `[checkout-api, checkout, api]` and a two-character CJK request into seven -// tokens — good for recall, but counting those as agreement means one word, or -// any CJK request at all, clears a floor meant to require two. +// The request as the user wrote it: one entry per whitespace-separated run. +// +// This is the unit the floor counts parts of the request in, and both the term +// floor and the alias floor have to count it the same way, so the split lives +// in one place rather than being repeated as a convention across six synced +// copies. +function words(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean); +} + +// Deduplicated, because the same word typed twice is not two parts agreeing. function queryTerms(query) { - return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); + return new Set(words(query)); } +// How much of the request genuinely agreed with this context. +// +// Evidence has to be independent on *both* sides, and each side alone is a +// bypass the other does not catch: +// +// - Counting parts of the request lets one concept spelled two ways pass as +// two. "what does user_id mean for a user?" is two words agreeing on the +// single token `user`. +// - Counting things agreed on lets one compound word pass as several. +// `tokenize` expands `checkout-api` into `[checkout-api, checkout, api]`, +// so a request of that one word would otherwise clear a floor of three. +// +// So what is counted is pairings: the largest set of (part of the request, +// thing it agreed on) pairs where no two pairs share either side. That is a +// maximum bipartite matching, and it is worth being exact rather than greedy +// about it — a greedy pass gives different answers for the same words in a +// different order, and "why does rewording the sentence change the route?" is +// not a question this should ever raise. +// +// One consequence is deliberate and documented: a script written without +// spaces — Chinese, or kanji-dense Japanese — is a single part of the request +// however long it is, so it can contribute at most one pairing and can never +// clear the floor. Auto-connect is therefore unreachable for those requests +// without an exact name or whole-request alias match, until this can segment +// them. That is a real limitation rather than a rounding error, and it is the +// conservative direction: the routing menu still answers, exactly as it does +// today for every user. It is narrower than "CJK" — Korean is written with +// spaces between eojeol and goes through the ordinary path. function agreeingTerms(candidate, query) { const carried = new Set( (candidate.matched ?? []).filter((term) => { @@ -147,13 +178,38 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } - let agreeing = 0; + const options = []; for (const term of queryTerms(query)) { - if (tokenize(term).some((token) => carried.has(token))) { - agreeing += 1; + const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); + if (agreed.length > 0) { + options.push(agreed); } } - return agreeing; + + // Kuhn's algorithm: give each part of the request something to agree on, + // letting an earlier one give up its choice whenever it has another. + const takenBy = new Map(); + const claim = (part, tried) => { + for (const token of options[part]) { + if (tried.has(token)) { + continue; + } + tried.add(token); + const holder = takenBy.get(token); + if (holder === undefined || claim(holder, tried)) { + takenBy.set(token, part); + return true; + } + } + return false; + }; + let paired = 0; + for (let part = 0; part < options.length; part += 1) { + if (claim(part, new Set())) { + paired += 1; + } + } + return paired; } // An alias is the one routing signal the user authored by hand, at the moment @@ -164,21 +220,17 @@ function agreeingTerms(candidate, query) { // alias therefore has to be the whole request; a longer one has to appear // contiguously in the request's tokens. // -// One word means one word the user typed, counted the way `queryTerms` counts -// the request — not the tokens the index derived from it. `tokenize` expands -// `checkout-api` into three and `user_id` into three, and reading that as a -// multi-word alias would reopen the bypass for every ticket id, service name -// and API version anyone is likely to register. +// One word means one word the user typed, counted by `words` exactly as the +// term floor counts the request — not the tokens the index derived from it. +// `tokenize` expands `checkout-api` into three and `user_id` into three, and +// reading that as a multi-word alias would reopen the bypass for every ticket +// id, service name and API version anyone is likely to register. // // It has to survive tokenizing as two, as well. `the api` is two words the user // typed, but `tokenize` drops the stopword and leaves one, and a one-token // contiguous check is just "does this word appear anywhere" — the very test the // first floor exists to prevent. `the API`, `our PR`, `how LM works` are how // people write these aliases down, so both floors have to hold. -function wordCount(text) { - return normalizeRoutingText(text).split(" ").filter(Boolean).length; -} - function matchesAlias(aliases, query) { const normalized = normalizeRoutingText(query); const queryTokens = tokenize(query); @@ -187,7 +239,7 @@ function matchesAlias(aliases, query) { if (aliasTokens.length === 0) { return false; } - if (wordCount(alias) < 2 || aliasTokens.length < 2) { + if (words(alias).length < 2 || aliasTokens.length < 2) { return normalizeRoutingText(alias) === normalized; } return queryTokens.some((_, start) => diff --git a/plugins/copilot/neatcontext/src/core/routing.mjs b/plugins/copilot/neatcontext/src/core/routing.mjs index a0a401c..266e040 100644 --- a/plugins/copilot/neatcontext/src/core/routing.mjs +++ b/plugins/copilot/neatcontext/src/core/routing.mjs @@ -47,6 +47,16 @@ const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; + +// Automatic routes are capped separately, and far shorter. +// +// They arrive at roughly one per new session, so a single shared cap would see +// them evict the manual selections this log exists for — and `familiarity` +// skipping them is exactly what would keep anyone from noticing the ground +// truth had drained away. Manual decisions keep their own hundred; the machine +// ones keep a short tail, which is all that is ever read back when working out +// why a session routed the way it did. +const MAX_AUTOMATIC_DECISIONS = 20; const MAX_SESSIONS = 20; // How long a refusal keeps counting for. @@ -192,7 +202,7 @@ async function writeRouting(state) { ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), - decisions: state.decisions.slice(-MAX_DECISIONS) + decisions: capDecisions(state.decisions) }, null, 2 @@ -201,6 +211,20 @@ async function writeRouting(state) { ); } +// Trimmed in two buckets rather than one, so the machine's own routes cannot +// push the user's out of the record. Merged back in time order afterwards, +// because everything downstream reads this as a chronology. +function capDecisions(decisions) { + const manual = []; + const automatic = []; + for (const decision of decisions) { + (decision?.automatic === true ? automatic : manual).push(decision); + } + return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( + (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) + ); +} + async function update(mutate) { const state = await readRouting(); const result = mutate(state); @@ -482,7 +506,7 @@ export function renderMenu(entries, { connectedId, mode, decision } = {}) { lines.push(""); const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push(tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); @@ -547,9 +571,12 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); + // Its own paragraph: it is the one line asking the model to stop and ask, + // and run together with the instructions around it that is what it stops + // looking like. const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push("", tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); diff --git a/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs b/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs index ce0eeb8..a6455c8 100644 --- a/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs @@ -125,18 +125,49 @@ export function normalizeRoutingText(text) { return text.trim().toLowerCase().replace(/\s+/g, " "); } -// The request split the way the user wrote it, deduplicated: one entry per -// whitespace-separated run. -// -// This is the unit the floor counts, and it has to be, because a token is not -// one. `tokenize` deliberately expands a single `checkout-api` into -// `[checkout-api, checkout, api]` and a two-character CJK request into seven -// tokens — good for recall, but counting those as agreement means one word, or -// any CJK request at all, clears a floor meant to require two. +// The request as the user wrote it: one entry per whitespace-separated run. +// +// This is the unit the floor counts parts of the request in, and both the term +// floor and the alias floor have to count it the same way, so the split lives +// in one place rather than being repeated as a convention across six synced +// copies. +function words(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean); +} + +// Deduplicated, because the same word typed twice is not two parts agreeing. function queryTerms(query) { - return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); + return new Set(words(query)); } +// How much of the request genuinely agreed with this context. +// +// Evidence has to be independent on *both* sides, and each side alone is a +// bypass the other does not catch: +// +// - Counting parts of the request lets one concept spelled two ways pass as +// two. "what does user_id mean for a user?" is two words agreeing on the +// single token `user`. +// - Counting things agreed on lets one compound word pass as several. +// `tokenize` expands `checkout-api` into `[checkout-api, checkout, api]`, +// so a request of that one word would otherwise clear a floor of three. +// +// So what is counted is pairings: the largest set of (part of the request, +// thing it agreed on) pairs where no two pairs share either side. That is a +// maximum bipartite matching, and it is worth being exact rather than greedy +// about it — a greedy pass gives different answers for the same words in a +// different order, and "why does rewording the sentence change the route?" is +// not a question this should ever raise. +// +// One consequence is deliberate and documented: a script written without +// spaces — Chinese, or kanji-dense Japanese — is a single part of the request +// however long it is, so it can contribute at most one pairing and can never +// clear the floor. Auto-connect is therefore unreachable for those requests +// without an exact name or whole-request alias match, until this can segment +// them. That is a real limitation rather than a rounding error, and it is the +// conservative direction: the routing menu still answers, exactly as it does +// today for every user. It is narrower than "CJK" — Korean is written with +// spaces between eojeol and goes through the ordinary path. function agreeingTerms(candidate, query) { const carried = new Set( (candidate.matched ?? []).filter((term) => { @@ -147,13 +178,38 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } - let agreeing = 0; + const options = []; for (const term of queryTerms(query)) { - if (tokenize(term).some((token) => carried.has(token))) { - agreeing += 1; + const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); + if (agreed.length > 0) { + options.push(agreed); } } - return agreeing; + + // Kuhn's algorithm: give each part of the request something to agree on, + // letting an earlier one give up its choice whenever it has another. + const takenBy = new Map(); + const claim = (part, tried) => { + for (const token of options[part]) { + if (tried.has(token)) { + continue; + } + tried.add(token); + const holder = takenBy.get(token); + if (holder === undefined || claim(holder, tried)) { + takenBy.set(token, part); + return true; + } + } + return false; + }; + let paired = 0; + for (let part = 0; part < options.length; part += 1) { + if (claim(part, new Set())) { + paired += 1; + } + } + return paired; } // An alias is the one routing signal the user authored by hand, at the moment @@ -164,21 +220,17 @@ function agreeingTerms(candidate, query) { // alias therefore has to be the whole request; a longer one has to appear // contiguously in the request's tokens. // -// One word means one word the user typed, counted the way `queryTerms` counts -// the request — not the tokens the index derived from it. `tokenize` expands -// `checkout-api` into three and `user_id` into three, and reading that as a -// multi-word alias would reopen the bypass for every ticket id, service name -// and API version anyone is likely to register. +// One word means one word the user typed, counted by `words` exactly as the +// term floor counts the request — not the tokens the index derived from it. +// `tokenize` expands `checkout-api` into three and `user_id` into three, and +// reading that as a multi-word alias would reopen the bypass for every ticket +// id, service name and API version anyone is likely to register. // // It has to survive tokenizing as two, as well. `the api` is two words the user // typed, but `tokenize` drops the stopword and leaves one, and a one-token // contiguous check is just "does this word appear anywhere" — the very test the // first floor exists to prevent. `the API`, `our PR`, `how LM works` are how // people write these aliases down, so both floors have to hold. -function wordCount(text) { - return normalizeRoutingText(text).split(" ").filter(Boolean).length; -} - function matchesAlias(aliases, query) { const normalized = normalizeRoutingText(query); const queryTokens = tokenize(query); @@ -187,7 +239,7 @@ function matchesAlias(aliases, query) { if (aliasTokens.length === 0) { return false; } - if (wordCount(alias) < 2 || aliasTokens.length < 2) { + if (words(alias).length < 2 || aliasTokens.length < 2) { return normalizeRoutingText(alias) === normalized; } return queryTokens.some((_, start) => diff --git a/plugins/kimi-code/neatcontext/src/core/routing.mjs b/plugins/kimi-code/neatcontext/src/core/routing.mjs index a0a401c..266e040 100644 --- a/plugins/kimi-code/neatcontext/src/core/routing.mjs +++ b/plugins/kimi-code/neatcontext/src/core/routing.mjs @@ -47,6 +47,16 @@ const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; + +// Automatic routes are capped separately, and far shorter. +// +// They arrive at roughly one per new session, so a single shared cap would see +// them evict the manual selections this log exists for — and `familiarity` +// skipping them is exactly what would keep anyone from noticing the ground +// truth had drained away. Manual decisions keep their own hundred; the machine +// ones keep a short tail, which is all that is ever read back when working out +// why a session routed the way it did. +const MAX_AUTOMATIC_DECISIONS = 20; const MAX_SESSIONS = 20; // How long a refusal keeps counting for. @@ -192,7 +202,7 @@ async function writeRouting(state) { ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), - decisions: state.decisions.slice(-MAX_DECISIONS) + decisions: capDecisions(state.decisions) }, null, 2 @@ -201,6 +211,20 @@ async function writeRouting(state) { ); } +// Trimmed in two buckets rather than one, so the machine's own routes cannot +// push the user's out of the record. Merged back in time order afterwards, +// because everything downstream reads this as a chronology. +function capDecisions(decisions) { + const manual = []; + const automatic = []; + for (const decision of decisions) { + (decision?.automatic === true ? automatic : manual).push(decision); + } + return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( + (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) + ); +} + async function update(mutate) { const state = await readRouting(); const result = mutate(state); @@ -482,7 +506,7 @@ export function renderMenu(entries, { connectedId, mode, decision } = {}) { lines.push(""); const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push(tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); @@ -547,9 +571,12 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); + // Its own paragraph: it is the one line asking the model to stop and ask, + // and run together with the instructions around it that is what it stops + // looking like. const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push("", tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); diff --git a/plugins/pi/neatcontext/src/core/routing-candidates.mjs b/plugins/pi/neatcontext/src/core/routing-candidates.mjs index ce0eeb8..a6455c8 100644 --- a/plugins/pi/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/pi/neatcontext/src/core/routing-candidates.mjs @@ -125,18 +125,49 @@ export function normalizeRoutingText(text) { return text.trim().toLowerCase().replace(/\s+/g, " "); } -// The request split the way the user wrote it, deduplicated: one entry per -// whitespace-separated run. -// -// This is the unit the floor counts, and it has to be, because a token is not -// one. `tokenize` deliberately expands a single `checkout-api` into -// `[checkout-api, checkout, api]` and a two-character CJK request into seven -// tokens — good for recall, but counting those as agreement means one word, or -// any CJK request at all, clears a floor meant to require two. +// The request as the user wrote it: one entry per whitespace-separated run. +// +// This is the unit the floor counts parts of the request in, and both the term +// floor and the alias floor have to count it the same way, so the split lives +// in one place rather than being repeated as a convention across six synced +// copies. +function words(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean); +} + +// Deduplicated, because the same word typed twice is not two parts agreeing. function queryTerms(query) { - return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); + return new Set(words(query)); } +// How much of the request genuinely agreed with this context. +// +// Evidence has to be independent on *both* sides, and each side alone is a +// bypass the other does not catch: +// +// - Counting parts of the request lets one concept spelled two ways pass as +// two. "what does user_id mean for a user?" is two words agreeing on the +// single token `user`. +// - Counting things agreed on lets one compound word pass as several. +// `tokenize` expands `checkout-api` into `[checkout-api, checkout, api]`, +// so a request of that one word would otherwise clear a floor of three. +// +// So what is counted is pairings: the largest set of (part of the request, +// thing it agreed on) pairs where no two pairs share either side. That is a +// maximum bipartite matching, and it is worth being exact rather than greedy +// about it — a greedy pass gives different answers for the same words in a +// different order, and "why does rewording the sentence change the route?" is +// not a question this should ever raise. +// +// One consequence is deliberate and documented: a script written without +// spaces — Chinese, or kanji-dense Japanese — is a single part of the request +// however long it is, so it can contribute at most one pairing and can never +// clear the floor. Auto-connect is therefore unreachable for those requests +// without an exact name or whole-request alias match, until this can segment +// them. That is a real limitation rather than a rounding error, and it is the +// conservative direction: the routing menu still answers, exactly as it does +// today for every user. It is narrower than "CJK" — Korean is written with +// spaces between eojeol and goes through the ordinary path. function agreeingTerms(candidate, query) { const carried = new Set( (candidate.matched ?? []).filter((term) => { @@ -147,13 +178,38 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } - let agreeing = 0; + const options = []; for (const term of queryTerms(query)) { - if (tokenize(term).some((token) => carried.has(token))) { - agreeing += 1; + const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); + if (agreed.length > 0) { + options.push(agreed); } } - return agreeing; + + // Kuhn's algorithm: give each part of the request something to agree on, + // letting an earlier one give up its choice whenever it has another. + const takenBy = new Map(); + const claim = (part, tried) => { + for (const token of options[part]) { + if (tried.has(token)) { + continue; + } + tried.add(token); + const holder = takenBy.get(token); + if (holder === undefined || claim(holder, tried)) { + takenBy.set(token, part); + return true; + } + } + return false; + }; + let paired = 0; + for (let part = 0; part < options.length; part += 1) { + if (claim(part, new Set())) { + paired += 1; + } + } + return paired; } // An alias is the one routing signal the user authored by hand, at the moment @@ -164,21 +220,17 @@ function agreeingTerms(candidate, query) { // alias therefore has to be the whole request; a longer one has to appear // contiguously in the request's tokens. // -// One word means one word the user typed, counted the way `queryTerms` counts -// the request — not the tokens the index derived from it. `tokenize` expands -// `checkout-api` into three and `user_id` into three, and reading that as a -// multi-word alias would reopen the bypass for every ticket id, service name -// and API version anyone is likely to register. +// One word means one word the user typed, counted by `words` exactly as the +// term floor counts the request — not the tokens the index derived from it. +// `tokenize` expands `checkout-api` into three and `user_id` into three, and +// reading that as a multi-word alias would reopen the bypass for every ticket +// id, service name and API version anyone is likely to register. // // It has to survive tokenizing as two, as well. `the api` is two words the user // typed, but `tokenize` drops the stopword and leaves one, and a one-token // contiguous check is just "does this word appear anywhere" — the very test the // first floor exists to prevent. `the API`, `our PR`, `how LM works` are how // people write these aliases down, so both floors have to hold. -function wordCount(text) { - return normalizeRoutingText(text).split(" ").filter(Boolean).length; -} - function matchesAlias(aliases, query) { const normalized = normalizeRoutingText(query); const queryTokens = tokenize(query); @@ -187,7 +239,7 @@ function matchesAlias(aliases, query) { if (aliasTokens.length === 0) { return false; } - if (wordCount(alias) < 2 || aliasTokens.length < 2) { + if (words(alias).length < 2 || aliasTokens.length < 2) { return normalizeRoutingText(alias) === normalized; } return queryTokens.some((_, start) => diff --git a/plugins/pi/neatcontext/src/core/routing.mjs b/plugins/pi/neatcontext/src/core/routing.mjs index a0a401c..266e040 100644 --- a/plugins/pi/neatcontext/src/core/routing.mjs +++ b/plugins/pi/neatcontext/src/core/routing.mjs @@ -47,6 +47,16 @@ const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; + +// Automatic routes are capped separately, and far shorter. +// +// They arrive at roughly one per new session, so a single shared cap would see +// them evict the manual selections this log exists for — and `familiarity` +// skipping them is exactly what would keep anyone from noticing the ground +// truth had drained away. Manual decisions keep their own hundred; the machine +// ones keep a short tail, which is all that is ever read back when working out +// why a session routed the way it did. +const MAX_AUTOMATIC_DECISIONS = 20; const MAX_SESSIONS = 20; // How long a refusal keeps counting for. @@ -192,7 +202,7 @@ async function writeRouting(state) { ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), - decisions: state.decisions.slice(-MAX_DECISIONS) + decisions: capDecisions(state.decisions) }, null, 2 @@ -201,6 +211,20 @@ async function writeRouting(state) { ); } +// Trimmed in two buckets rather than one, so the machine's own routes cannot +// push the user's out of the record. Merged back in time order afterwards, +// because everything downstream reads this as a chronology. +function capDecisions(decisions) { + const manual = []; + const automatic = []; + for (const decision of decisions) { + (decision?.automatic === true ? automatic : manual).push(decision); + } + return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( + (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) + ); +} + async function update(mutate) { const state = await readRouting(); const result = mutate(state); @@ -482,7 +506,7 @@ export function renderMenu(entries, { connectedId, mode, decision } = {}) { lines.push(""); const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push(tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); @@ -547,9 +571,12 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); + // Its own paragraph: it is the one line asking the model to stop and ask, + // and run together with the instructions around it that is what it stops + // looking like. const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push("", tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); diff --git a/shared/core/routing-candidates.mjs b/shared/core/routing-candidates.mjs index ce0eeb8..a6455c8 100644 --- a/shared/core/routing-candidates.mjs +++ b/shared/core/routing-candidates.mjs @@ -125,18 +125,49 @@ export function normalizeRoutingText(text) { return text.trim().toLowerCase().replace(/\s+/g, " "); } -// The request split the way the user wrote it, deduplicated: one entry per -// whitespace-separated run. -// -// This is the unit the floor counts, and it has to be, because a token is not -// one. `tokenize` deliberately expands a single `checkout-api` into -// `[checkout-api, checkout, api]` and a two-character CJK request into seven -// tokens — good for recall, but counting those as agreement means one word, or -// any CJK request at all, clears a floor meant to require two. +// The request as the user wrote it: one entry per whitespace-separated run. +// +// This is the unit the floor counts parts of the request in, and both the term +// floor and the alias floor have to count it the same way, so the split lives +// in one place rather than being repeated as a convention across six synced +// copies. +function words(text) { + return normalizeRoutingText(text).split(" ").filter(Boolean); +} + +// Deduplicated, because the same word typed twice is not two parts agreeing. function queryTerms(query) { - return new Set(normalizeRoutingText(query).split(" ").filter(Boolean)); + return new Set(words(query)); } +// How much of the request genuinely agreed with this context. +// +// Evidence has to be independent on *both* sides, and each side alone is a +// bypass the other does not catch: +// +// - Counting parts of the request lets one concept spelled two ways pass as +// two. "what does user_id mean for a user?" is two words agreeing on the +// single token `user`. +// - Counting things agreed on lets one compound word pass as several. +// `tokenize` expands `checkout-api` into `[checkout-api, checkout, api]`, +// so a request of that one word would otherwise clear a floor of three. +// +// So what is counted is pairings: the largest set of (part of the request, +// thing it agreed on) pairs where no two pairs share either side. That is a +// maximum bipartite matching, and it is worth being exact rather than greedy +// about it — a greedy pass gives different answers for the same words in a +// different order, and "why does rewording the sentence change the route?" is +// not a question this should ever raise. +// +// One consequence is deliberate and documented: a script written without +// spaces — Chinese, or kanji-dense Japanese — is a single part of the request +// however long it is, so it can contribute at most one pairing and can never +// clear the floor. Auto-connect is therefore unreachable for those requests +// without an exact name or whole-request alias match, until this can segment +// them. That is a real limitation rather than a rounding error, and it is the +// conservative direction: the routing menu still answers, exactly as it does +// today for every user. It is narrower than "CJK" — Korean is written with +// spaces between eojeol and goes through the ordinary path. function agreeingTerms(candidate, query) { const carried = new Set( (candidate.matched ?? []).filter((term) => { @@ -147,13 +178,38 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } - let agreeing = 0; + const options = []; for (const term of queryTerms(query)) { - if (tokenize(term).some((token) => carried.has(token))) { - agreeing += 1; + const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); + if (agreed.length > 0) { + options.push(agreed); } } - return agreeing; + + // Kuhn's algorithm: give each part of the request something to agree on, + // letting an earlier one give up its choice whenever it has another. + const takenBy = new Map(); + const claim = (part, tried) => { + for (const token of options[part]) { + if (tried.has(token)) { + continue; + } + tried.add(token); + const holder = takenBy.get(token); + if (holder === undefined || claim(holder, tried)) { + takenBy.set(token, part); + return true; + } + } + return false; + }; + let paired = 0; + for (let part = 0; part < options.length; part += 1) { + if (claim(part, new Set())) { + paired += 1; + } + } + return paired; } // An alias is the one routing signal the user authored by hand, at the moment @@ -164,21 +220,17 @@ function agreeingTerms(candidate, query) { // alias therefore has to be the whole request; a longer one has to appear // contiguously in the request's tokens. // -// One word means one word the user typed, counted the way `queryTerms` counts -// the request — not the tokens the index derived from it. `tokenize` expands -// `checkout-api` into three and `user_id` into three, and reading that as a -// multi-word alias would reopen the bypass for every ticket id, service name -// and API version anyone is likely to register. +// One word means one word the user typed, counted by `words` exactly as the +// term floor counts the request — not the tokens the index derived from it. +// `tokenize` expands `checkout-api` into three and `user_id` into three, and +// reading that as a multi-word alias would reopen the bypass for every ticket +// id, service name and API version anyone is likely to register. // // It has to survive tokenizing as two, as well. `the api` is two words the user // typed, but `tokenize` drops the stopword and leaves one, and a one-token // contiguous check is just "does this word appear anywhere" — the very test the // first floor exists to prevent. `the API`, `our PR`, `how LM works` are how // people write these aliases down, so both floors have to hold. -function wordCount(text) { - return normalizeRoutingText(text).split(" ").filter(Boolean).length; -} - function matchesAlias(aliases, query) { const normalized = normalizeRoutingText(query); const queryTokens = tokenize(query); @@ -187,7 +239,7 @@ function matchesAlias(aliases, query) { if (aliasTokens.length === 0) { return false; } - if (wordCount(alias) < 2 || aliasTokens.length < 2) { + if (words(alias).length < 2 || aliasTokens.length < 2) { return normalizeRoutingText(alias) === normalized; } return queryTokens.some((_, start) => diff --git a/shared/core/routing.mjs b/shared/core/routing.mjs index a0a401c..266e040 100644 --- a/shared/core/routing.mjs +++ b/shared/core/routing.mjs @@ -47,6 +47,16 @@ const SCHEMA = 2; const MAX_USE_WHEN = 240; const MAX_ALIASES = 12; const MAX_DECISIONS = 100; + +// Automatic routes are capped separately, and far shorter. +// +// They arrive at roughly one per new session, so a single shared cap would see +// them evict the manual selections this log exists for — and `familiarity` +// skipping them is exactly what would keep anyone from noticing the ground +// truth had drained away. Manual decisions keep their own hundred; the machine +// ones keep a short tail, which is all that is ever read back when working out +// why a session routed the way it did. +const MAX_AUTOMATIC_DECISIONS = 20; const MAX_SESSIONS = 20; // How long a refusal keeps counting for. @@ -192,7 +202,7 @@ async function writeRouting(state) { ...(MODES.includes(mode) ? { mode } : {}), sessions: Object.fromEntries(sessions), declines: pruneDeclines(state.declines, Date.now()), - decisions: state.decisions.slice(-MAX_DECISIONS) + decisions: capDecisions(state.decisions) }, null, 2 @@ -201,6 +211,20 @@ async function writeRouting(state) { ); } +// Trimmed in two buckets rather than one, so the machine's own routes cannot +// push the user's out of the record. Merged back in time order afterwards, +// because everything downstream reads this as a chronology. +function capDecisions(decisions) { + const manual = []; + const automatic = []; + for (const decision of decisions) { + (decision?.automatic === true ? automatic : manual).push(decision); + } + return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( + (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) + ); +} + async function update(mutate) { const state = await readRouting(); const result = mutate(state); @@ -482,7 +506,7 @@ export function renderMenu(entries, { connectedId, mode, decision } = {}) { lines.push(""); const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push(tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); @@ -547,9 +571,12 @@ export function renderShortlist(entries, { connectedId, mode, decision } = {}) { ? "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — that is a reason to stay where you are, not to reach for the closest one here." : "These are the contexts on this machine whose own description matched the request, best first. Others exist and did not match — so if none of these covers the request, say the store does not have it rather than reaching for the closest one here." ); + // Its own paragraph: it is the one line asking the model to stop and ask, + // and run together with the instructions around it that is what it stops + // looking like. const tie = tieNote(decision); if (tie) { - lines.push(tie); + lines.push("", tie, ""); } lines.push(...routingInstructions(mode, Boolean(connectedId))); return lines.join("\n"); diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index a84bfe7..f789324 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -8,7 +8,7 @@ import assert from "node:assert/strict"; import { spawn } from "node:child_process"; -import { mkdir, mkdtemp, readFile, readdir, rm, stat, writeFile } from "node:fs/promises"; +import { chmod, mkdir, mkdtemp, readFile, readdir, rm, stat, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import test from "node:test"; @@ -238,6 +238,86 @@ async function createContext( return result; } +// An auto-connection is a context switch that never went through `use_context`, +// so the teardown it owes is the same one a switch owes: whatever the previous +// context had running, stopped. Clearing the local arrays only hides those +// tools from the session — the child processes behind them keep running until +// the bridge exits, which for a long-lived server is never. +test("Copilot stops the previous context's extensions when it auto-connects", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-dispose-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect-dispose"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Payments", { sessionId, useWhen: "payment reconciliation" }); + await createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + + const connect = await runNode(cli, ["use", "Payments"], { env }); + assert.match(connect.stdout, /Connected the "Payments" context/); + const declared = await runNode( + cli, + ["extensions", "add", "pagerduty", "--capability", "Read incidents.", "--tools", "get_incident"], + { env } + ); + assert.match(declared.stdout, /now expects the "pagerduty" extension/); + + const pidFile = path.join(home.directory, "extension.pid"); + await writeFile( + path.join(home.directory, "extensions.json"), + `${JSON.stringify({ + schema: 1, + extensions: { + pagerduty: { + command: process.execPath, + args: [path.join(here, "fake-extension-server.mjs")], + env: { FAKE_MCP_PID_FILE: pidFile } + } + } + })}\n`, + "utf8" + ); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + const listed = await session.call({ jsonrpc: "2.0", id: 2, method: "tools/list" }); + assert.ok(listed.result.tools.some((tool) => tool.name === "pagerduty__get_incident")); + const pid = Number(await readFile(pidFile, "utf8")); + assert.ok(Number.isInteger(pid) && pid > 0); + assert.doesNotThrow(() => process.kill(pid, 0), "the extension server is running"); + t.after(() => { + try { + process.kill(pid); + } catch { + // Already gone, which is the point of the test. + } + }); + + // Out of band, the way `/neatcontext:disconnect` does it: the next call finds + // nothing connected, and a request that plainly belongs elsewhere. + const disconnect = await runNode(cli, ["disconnect"], { env }); + assert.equal(disconnect.code, 0); + const auto = await session.call( + toolCall(3, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.match(auto.result.content[0].text, /Checkout incident/); + + for (let attempt = 0; attempt < 50; attempt += 1) { + try { + process.kill(pid, 0); + } catch { + return; // gone, which is what this test is for + } + await new Promise((resolve) => setTimeout(resolve, 100)); + } + assert.fail("the previous context's extension server outlived the auto-connection"); +}); + test("Copilot plugin manifest is complete, version-aligned, and listed in the marketplace", async () => { const [pluginText, marketplaceText, packageText, bridgeText, readme, copilotReadme] = await Promise.all([ @@ -908,6 +988,17 @@ test("Copilot narrows the routing menu to the request", async (t) => { ); assert.match(unmatched.result.content[0].text, /## Contexts available on this machine/); assert.match(unmatched.result.content[0].text, /Docker container/); + + // The shape users actually type. Narrowing a full sentence is the case worth + // guarding, and it needs a connected session to test on its own terms: + // unconnected, this query auto-connects and never renders a shortlist. + await session.call(toolCall(6, "use_context", { context: "Session drift", requested: true })); + const sentence = await session.call( + toolCall(7, "get_context", { query: "why is checkout throwing 5xx" }) + ); + assert.match(sentence.result.content[0].text, /## Contexts that match what the user just asked/); + assert.match(sentence.result.content[0].text, /INC-1001 checkout/); + assert.ok(!sentence.result.content[0].text.includes("Docker container")); }); // The interaction auto-connect actually introduced, which nothing else covers: @@ -1283,6 +1374,144 @@ test("Copilot get_context preserves ask mode for a clear match", async (t) => { assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); }); +// A refusal is per-session in `switchPolicy`, and globally only a score +// discount — which cannot change an outcome in a store too small to have a +// rival. Through the model that was survivable, because `use_context` +// announces the switch and the user can say no again. Acting unasked removes +// the announcement, so the refusal has to be able to stop it outright. +test("Copilot get_context does not auto-connect a context declined in another session", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-declined-global-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + await createContext(home, "Checkout incident", { + sessionId: "monday", + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + + const monday = rpcSession({ ...home.env, NEATCONTEXT_SESSION_ID: "monday" }); + sessions.push(monday); + await monday.call(initialize(1)); + const declined = await monday.call( + toolCall(2, "use_context", { context: "Checkout incident", declined: true }) + ); + assert.equal(declined.result.isError, false); + + // A different session entirely, with a clean slate of its own. + const wednesday = rpcSession({ ...home.env, NEATCONTEXT_SESSION_ID: "wednesday" }); + sessions.push(wednesday); + await wednesday.call(initialize(1)); + const response = await wednesday.call( + toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); +}); + +// The near-tie is a property of the ranking, not of whether this particular +// call was allowed to act on it. Read off the auto-connect path alone, the note +// went silent in every situation that path bails out of — which is most of +// them, and a connected session on a small store is the commonest. +test("Copilot names a near-tie even when auto-connect was never on the table", async (t) => { + const home = await isolatedHome("neatcontext-copilot-tie-connected-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-tie-connected"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Somewhere else", { sessionId, useWhen: "unrelated bookkeeping" }); + await createContext(home, "Codex plugin", { + sessionId, + useWhen: "plugin packaging and marketplace manifests" + }); + await createContext(home, "Kimi plugin", { + sessionId, + useWhen: "plugin packaging and marketplace manifests" + }); + const use = await runNode(cli, ["use", "Somewhere else"], { env }); + assert.match(use.stdout, /Connected the "Somewhere else" context/); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + const response = await session.call( + toolCall(2, "get_context", { query: "plugin packaging and marketplace manifests" }) + ); + assert.match(response.result.content[0].text, /match the request about equally well/); + assert.match(response.result.content[0].text, /\*\*Codex plugin\*\* and \*\*Kimi plugin\*\*/); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); +}); + +// A selection pointing at a context that is gone is nothing connected — +// `readSelection` deletes the file on its way out. Read as connected, the one +// call the user most needs a straight answer on says nothing is connected while +// carrying the guards written for a session that has somewhere to leave. +test("Copilot treats a stale selection as nothing connected", async (t) => { + const home = await isolatedHome("neatcontext-copilot-stale-selection-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-stale-selection"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + + // Written after the handshake on purpose: `readSelection` clears a stale file + // as it reads it, so a selection planted before the session starts is gone by + // the time the call under test arrives. + const selectionFile = path.join(home.directory, "plugin-sessions", `${sessionId}.json`); + await mkdir(path.dirname(selectionFile), { recursive: true }); + await writeFile( + selectionFile, + JSON.stringify({ contextId: "gone", contextName: "Gone" }), + "utf8" + ); + + const response = await session.call(toolCall(2, "get_context")); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + assert.doesNotMatch(response.result.content[0].text, /no longer exists on disk/); +}); + +// The connection is made by `applySelection`; the decision log is bookkeeping +// that follows it. Written in the other order, a log failure produced a pass +// that said nothing was connected and a disk that said otherwise. +test("Copilot reports the connection it made even when the decision log cannot be written", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-log-fails-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-auto-connect-log-fails"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + // Read-only where the routing state file goes: it still reads, so the card + // that makes this context matchable survives, but every write to it fails — + // which is what a hostile permission set looks like from here. + const routingFile = path.join(home.directory, "plugin-routing.json"); + await chmod(routingFile, 0o444); + t.after(() => chmod(routingFile, 0o644).catch(() => undefined)); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + const response = await session.call( + toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.match(response.result.content[0].text, /Checkout incident/); + assert.doesNotMatch(response.result.content[0].text, /No NeatContext Context is connected/); +}); + test("Copilot get_context never auto-switches an existing connection", async (t) => { const home = await isolatedHome("neatcontext-copilot-auto-switch-"); const sessions = []; diff --git a/tests/fake-extension-server.mjs b/tests/fake-extension-server.mjs index bc8b539..9eb4d36 100644 --- a/tests/fake-extension-server.mjs +++ b/tests/fake-extension-server.mjs @@ -12,7 +12,10 @@ // FAKE_MCP_ERROR_TOOL return a JSON-RPC error when this tool is called // FAKE_MCP_NOISE write a non-JSON banner line to stdout first // FAKE_MCP_ECHO_ENV report this variable's value from the `echo_env` tool +// FAKE_MCP_PID_FILE write this process's pid here, so a test that cares +// whether the child outlived its host can go and look +import { writeFileSync } from "node:fs"; import readline from "node:readline"; const required = process.env.FAKE_MCP_REQUIRE_ENV; @@ -21,6 +24,10 @@ if (required && !process.env[required]) { process.exit(1); } +if (process.env.FAKE_MCP_PID_FILE) { + writeFileSync(process.env.FAKE_MCP_PID_FILE, String(process.pid), "utf8"); +} + const toolNames = (process.env.FAKE_MCP_TOOLS ?? "search_incidents,get_incident").split(","); const hang = process.env.FAKE_MCP_HANG ?? ""; const exitAfter = Number(process.env.FAKE_MCP_EXIT_AFTER ?? "0"); diff --git a/tests/routing-confidence.test.mjs b/tests/routing-confidence.test.mjs index 6e98c65..2cd441d 100644 --- a/tests/routing-confidence.test.mjs +++ b/tests/routing-confidence.test.mjs @@ -193,6 +193,81 @@ describe("isConfidentMatch", () => { assert.equal(isConfidentMatch(leader, "订单延迟 排查"), false, "排查 did not match"); }); + it("leaves a no-space script unable to reach the floor, deliberately", () => { + // A script written without spaces is one part of the request however long + // it runs, so it can supply at most one pairing. Auto-connect is therefore + // out of reach for these requests until this can segment them, and the + // escape hatches below are the whole of what is left. This is a decision, + // not an oversight: the routing menu still answers, which is what every + // user gets today. + const leader = candidate( + "订单系统", + hit("订单"), + hit("延迟"), + hit("排查"), + hit("步骤"), + hit("订"), + hit("单") + ); + assert.equal(isConfidentMatch(leader, "订单延迟排查步骤"), false); + + // Naming the context still works, and so does an alias the user wrote, + // when it is the whole request. + assert.equal(isConfidentMatch(candidate("订单排查"), "订单排查"), true); + assert.equal( + isConfidentMatch(leader, "订单排查", { aliases: ["订单排查"] }), + true + ); + }); + + it("does not black out Korean, which is written with spaces", () => { + // The limitation above is about scripts with no spaces, not about CJK: a + // Korean request separates its eojeol and goes through the ordinary path. + const leader = candidate("주문 지연", hit("주문"), hit("지연")); + assert.equal(isConfidentMatch(leader, "주문 지연"), true); + }); + + it("does not let one concept spelled two ways count as two", () => { + // `user_id` tokenizes to [user_id, user, id] and `user?` to [user], so both + // words agree on the single carried token `user`. Two parts of the request, + // one thing agreed on — which is one piece of evidence, not two. + assert.equal( + isConfidentMatch(candidate("Users", hit("user")), "what does user_id mean for a user?"), + false + ); + assert.equal( + isConfidentMatch(candidate("Docker", hit("docker")), "how do I run docker in docker-compose"), + false + ); + assert.equal( + isConfidentMatch(candidate("Checkout API", hit("api")), "is the api part of checkout-api?"), + false + ); + // Two compounds sharing their only carried token is still one concept. + assert.equal( + isConfidentMatch(candidate("Docker", hit("docker")), "docker-compose docker-swarm"), + false + ); + // Two spellings that are genuinely two things still count as two. + assert.equal( + isConfidentMatch(candidate("Users", hit("user"), hit("users")), "user users"), + true + ); + }); + + it("pairs the same way whichever order the words arrive in", () => { + // `alpha-beta` can be spent on either token, so a first-come pairing counts + // this as two one way round and one the other. Rewording a sentence must + // not change where it routes. + const leader = candidate("Alpha", hit("alpha"), hit("beta")); + assert.equal(isConfidentMatch(leader, "alpha alpha-beta"), true); + assert.equal(isConfidentMatch(leader, "alpha-beta alpha"), true); + + const single = candidate("Alpha", hit("alpha")); + assert.equal(isConfidentMatch(single, "alpha alpha-beta"), false); + assert.equal(isConfidentMatch(single, "alpha-beta alpha"), false); + }); + it("does not let two filenames stand in for what a context is for", () => { const leader = candidate("Payments", hit("deploy", "files"), hit("runbook", "files")); assert.equal(isConfidentMatch(leader, "where is the deploy runbook?"), false); @@ -379,6 +454,22 @@ describe("renderMenu with a decision", () => { assert.ok(!routing.renderMenu(entries, { mode: "auto", decision: clear }).includes("equally well")); assert.ok(!routing.renderMenu(entries, { mode: "auto" }).includes("equally well")); }); + + // The note is its own paragraph. Run into the guidance that follows it, the + // two read as one sentence about the wrong thing. + it("stands the near-tie note apart from the guidance around it", () => { + const decision = assess(scored); + for (const text of [ + routing.renderMenu(entries, { mode: "auto", decision }), + routing.renderShortlist(entries, { mode: "auto", decision }) + ]) { + const lines = text.split("\n"); + const at = lines.findIndex((line) => line.includes("match the request about equally well")); + assert.ok(at > 0, "the note is there to be spaced"); + assert.equal(lines[at - 1], ""); + assert.equal(lines[at + 1], ""); + } + }); }); describe("the bridge decides from real scores", () => { diff --git a/tests/routing.test.mjs b/tests/routing.test.mjs index 4a799a3..75cbcb8 100644 --- a/tests/routing.test.mjs +++ b/tests/routing.test.mjs @@ -169,6 +169,53 @@ describe("routing metadata", () => { assert.equal(updated.decisions.at(-1).to, "Payments"); }); + it("keeps the machine's own routes from evicting the user's", async () => { + // Automatic routes arrive at about one per new session, so a shared cap + // would drain the log of exactly the manual selections `familiarity` reads + // — and `familiarity` skipping them is what would keep anyone from + // noticing. Two buckets, merged back in time order. + const record = await create("Capped", "cap check"); + const at = (minute) => new Date(Date.UTC(2026, 0, 1, 0, minute)).toISOString(); + for (let index = 0; index < 40; index += 1) { + await routing.noteDecision({ + sessionId: `auto-${index}`, + from: null, + to: record.name, + at: at(index), + automatic: true + }); + } + await routing.noteDecision({ + sessionId: "human", + from: null, + to: record.name, + at: at(100), + requested: true + }); + for (let index = 0; index < 40; index += 1) { + await routing.noteDecision({ + sessionId: `auto-late-${index}`, + from: null, + to: record.name, + at: at(200 + index), + automatic: true + }); + } + + const state = await routing.readRouting(); + const automatic = state.decisions.filter((decision) => decision.automatic === true); + const manual = state.decisions.filter((decision) => decision.automatic !== true); + assert.equal(automatic.length, 20, "automatic routes keep only a short tail"); + assert.equal( + manual.some((decision) => decision.sessionId === "human"), + true, + "the one manual decision survived 80 automatic ones" + ); + // Still a chronology, which is how everything downstream reads it. + const times = state.decisions.map((decision) => Date.parse(decision.at)); + assert.deepEqual(times, [...times].sort((left, right) => left - right)); + }); + it("enforces auto, ask, manual, declined, and already-connected policies", () => { const base = { mode: "ask", sessions: {} }; assert.equal( From 1d6dc92cab5a398c0d657e9a898f145cad345d83 Mon Sep 17 00:00:00 2001 From: Jingyu Ma Date: Wed, 12 Aug 2026 14:33:01 -0700 Subject: [PATCH 5/5] Address round-three review on the auto-connect path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Eight findings from the maintainer's third pass. The confidence floor counted a compound and the parts `tokenize` derived from it as separate evidence. `rank` returns every token the index holds, so a description containing `user_id` carries `user_id`, `user` and `id`, and a request naming that one concept twice cleared a floor of two. The carried set now collapses by derivation before pairing, which closes it without touching the matching itself. The tests that guarded this were fixtured with a shape `rank` never produces, so they are re-fixtured, and a new case runs the whole rule against a real index. An automatic decision no longer creates a session record. Records hold the per-session mode override and what the user declined there, and every writer used to need a person or a model; auto-connect fires about once per new session, so it turned the session cap into a shredder — twenty windows connecting themselves elsewhere evicted a window pinned to manual and it silently started routing again. The near-tie note is assessed over the shortlist it is printed beneath rather than the whole corpus, so it can no longer name contexts the model was never shown. A host that publishes no session id now says so, instead of going quiet in a way that reads as "nothing matched". Manual mode stops ranking for a list neither renderer will read. A refusal bars connecting unasked for one half-life, named, rather than for as long as `declineFactor` stays under 1 — six weeks, by the end of which it is a one-percent discount. A decision whose timestamp will not parse stays where it was instead of being relocated to 1970 on every write. The unwritable-routing-file test used `chmod 0o444`, which denies nothing to root and only sets an attribute on Windows, and it matched on a card stored in the file it was destroying. It now uses a directory and matches on the name. --- .../src/core/routing-candidates.mjs | 34 +++ .../plugins/neatcontext/src/core/routing.mjs | 87 +++++++- .../src/core/routing-candidates.mjs | 34 +++ .../neatcontext/src/core/routing.mjs | 87 +++++++- .../neatcontext/src/copilot/mcp-bridge.mjs | 63 +++++- .../src/core/routing-candidates.mjs | 34 +++ .../copilot/neatcontext/src/core/routing.mjs | 87 +++++++- .../src/core/routing-candidates.mjs | 34 +++ .../neatcontext/src/core/routing.mjs | 87 +++++++- .../src/core/routing-candidates.mjs | 34 +++ plugins/pi/neatcontext/src/core/routing.mjs | 87 +++++++- shared/core/routing-candidates.mjs | 34 +++ shared/core/routing.mjs | 87 +++++++- tests/copilot-plugin.test.mjs | 202 +++++++++++++++++- tests/routing-confidence.test.mjs | 111 +++++++++- tests/routing.test.mjs | 110 ++++++++++ 16 files changed, 1163 insertions(+), 49 deletions(-) diff --git a/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs b/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs index a6455c8..82c1751 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/routing-candidates.mjs @@ -178,6 +178,40 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } + + // A compound and the parts `tokenize` derived from it are one thing this + // context knows about, and they have to arrive on this side as one. + // + // The pairing below is independent on both sides, but `matched` is not: + // `rank` returns every token the index holds, and a description containing + // `user_id` indexes `user_id`, `user` and `id`. All three come back, so a + // request that named the one concept twice found two distinct things waiting + // to be paired with — "what does user_id mean for a user?" cleared a floor of + // two on `user_id` and `user`, which is the very bypass this floor exists to + // close, arriving on the other side of it. + // + // The longest spelling is what is kept, and only its own derived parts are + // dropped: `user` and `users` derive from neither each other nor a common + // compound, so two things that really are two still count as two. Deletion is + // by derivation rather than by substring for the same reason — `id` inside + // `identity` is a different word, and dropping it would silently disarm the + // floor for any context whose description happens to contain a longer word. + // + // One direction is given up deliberately. A description that carries a + // derived part as a word of its own — "the `user_id` in the `user` table" — + // is indistinguishable here from one that only carries the compound, and both + // collapse to one. That costs an auto-connection on a description that really + // did name two things; the alternative costs a session re-grounded on a + // context it only half matched, unannounced. On the one surface that acts + // without asking, the miss is the cheaper mistake. + for (const term of [...carried]) { + for (const part of tokenize(term)) { + if (part !== term) { + carried.delete(part); + } + } + } + const options = []; for (const term of queryTerms(query)) { const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); diff --git a/codex-marketplace/plugins/neatcontext/src/core/routing.mjs b/codex-marketplace/plugins/neatcontext/src/core/routing.mjs index 266e040..a7a2931 100644 --- a/codex-marketplace/plugins/neatcontext/src/core/routing.mjs +++ b/codex-marketplace/plugins/neatcontext/src/core/routing.mjs @@ -98,6 +98,10 @@ const STICKY_BOOST = 1.35; const DECLINE_HALF_LIFE_DAYS = 14; const DECLINE_LIFETIME_DAYS = 42; + +// How long a refusal bars connecting unasked, as opposed to merely discounting +// the score. See `hasLiveDecline` for why the two differ. +const LIVE_DECLINE_DAYS = DECLINE_HALF_LIFE_DAYS; const DECLINE_WEIGHT = 0.4; const MAX_DECLINE_COUNT = 10; const DAY_MS = 24 * 60 * 60 * 1000; @@ -220,9 +224,45 @@ function capDecisions(decisions) { for (const decision of decisions) { (decision?.automatic === true ? automatic : manual).push(decision); } - return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( - (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) - ); + return mergeByTime(manual.slice(-MAX_DECISIONS), automatic.slice(-MAX_AUTOMATIC_DECISIONS)); +} + +// Where a decision sits in the log, for a decision whose `at` will not parse. +// +// It sits where it already was. `Date.parse` returns NaN for a hand-edited +// entry, a half-written one, or one from a future writer that spells the field +// differently, and `NaN || 0` reads that as the first of January 1970 — which +// sorts it to the front of the log and, because this runs on every write, +// leaves it there for good. One unreadable timestamp then permanently rewrites +// the chronology `familiarity` and every "why did it route that way?" read +// back off this file. +// +// Each bucket is already in the order it was appended, so carrying the last +// timestamp forward within it keeps such an entry beside the decisions it was +// actually made among — the only evidence about it that is left. +function timeKeys(bucket) { + let last = -Infinity; + return bucket.map((decision) => { + const parsed = Date.parse(decision?.at); + if (Number.isFinite(parsed)) { + last = parsed; + } + return last; + }); +} + +// A merge rather than a sort, because both sides arrive ordered and a merge is +// the one way to interleave them that cannot move anything within its own side. +function mergeByTime(left, right) { + const leftKeys = timeKeys(left); + const rightKeys = timeKeys(right); + const merged = []; + let l = 0; + let r = 0; + while (l < left.length && r < right.length) { + merged.push(leftKeys[l] <= rightKeys[r] ? left[l++] : right[r++]); + } + return [...merged, ...left.slice(l), ...right.slice(r)]; } async function update(mutate) { @@ -395,6 +435,30 @@ export function declineFactor(state, contextId, now = new Date()) { return (1 - strength) ** entry.count; } +// Whether a refusal is recent enough to forbid connecting without being asked. +// +// Deliberately not `declineFactor(...) < 1`. That reads a veto off a floating +// point comparison, and it is true for the whole six weeks the multiplier takes +// to decay: at day 41 the factor is about 0.99, a discount the ranking treats +// as no discount at all, while the gate was treating it as an absolute bar. It +// also leaves the threshold — the thing a reader most needs to know — implicit +// in a value nothing names. +// +// One half-life is where the line goes. For that long a refusal still carries +// most of the weight it was given, and connecting unasked is the one route the +// user gets no chance to stop. Past it, the multiplier is the whole answer: a +// faded refusal is a hint, and the ranking is where hints belong. +// +// A timestamp from the future counts as live, matching how `declineFactor` +// floors the age at zero. Both err towards not acting. +export function hasLiveDecline(state, contextId, now = new Date()) { + const at = Date.parse(state?.declines?.[contextId]?.at); + if (!Number.isFinite(at)) { + return false; + } + return (now.getTime() - at) / DAY_MS < LIVE_DECLINE_DAYS; +} + // How much this machine's own history argues for a context: a multiplier at or // above 1, never below, because this is a hint and not evidence. // @@ -456,7 +520,22 @@ export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); const id = entry.sessionId; - if (id) { + // A session record holds what the user chose for that window: its `mode` + // override and the contexts they declined in it. Every writer of one used + // to need a person or a model to act — `setMode`, `noteDeclined`, or a + // `use_context` call. An automatic route needs neither, and it arrives at + // about one per new session, so creating a record for one turned the + // `MAX_SESSIONS` cap into a shredder: twenty windows auto-connecting + // elsewhere would evict the record of a window where somebody had run + // `/neatcontext:mode manual`, `resolveMode` would fall through to the + // default — which is `auto` — and a session where the user had turned + // routing off would start routing itself again, silently, having also + // forgotten what they declined there. + // + // So an automatic decision keeps a record that already exists up to date, + // and creates none. The log still has the route: `decisions` is where a + // machine route belongs, and it is capped in its own bucket. + if (id && (state.sessions[id] || entry.automatic !== true)) { const session = state.sessions[id] ?? {}; state.sessions[id] = { ...session, diff --git a/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs b/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs index a6455c8..82c1751 100644 --- a/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/claude-code/neatcontext/src/core/routing-candidates.mjs @@ -178,6 +178,40 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } + + // A compound and the parts `tokenize` derived from it are one thing this + // context knows about, and they have to arrive on this side as one. + // + // The pairing below is independent on both sides, but `matched` is not: + // `rank` returns every token the index holds, and a description containing + // `user_id` indexes `user_id`, `user` and `id`. All three come back, so a + // request that named the one concept twice found two distinct things waiting + // to be paired with — "what does user_id mean for a user?" cleared a floor of + // two on `user_id` and `user`, which is the very bypass this floor exists to + // close, arriving on the other side of it. + // + // The longest spelling is what is kept, and only its own derived parts are + // dropped: `user` and `users` derive from neither each other nor a common + // compound, so two things that really are two still count as two. Deletion is + // by derivation rather than by substring for the same reason — `id` inside + // `identity` is a different word, and dropping it would silently disarm the + // floor for any context whose description happens to contain a longer word. + // + // One direction is given up deliberately. A description that carries a + // derived part as a word of its own — "the `user_id` in the `user` table" — + // is indistinguishable here from one that only carries the compound, and both + // collapse to one. That costs an auto-connection on a description that really + // did name two things; the alternative costs a session re-grounded on a + // context it only half matched, unannounced. On the one surface that acts + // without asking, the miss is the cheaper mistake. + for (const term of [...carried]) { + for (const part of tokenize(term)) { + if (part !== term) { + carried.delete(part); + } + } + } + const options = []; for (const term of queryTerms(query)) { const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); diff --git a/plugins/claude-code/neatcontext/src/core/routing.mjs b/plugins/claude-code/neatcontext/src/core/routing.mjs index 266e040..a7a2931 100644 --- a/plugins/claude-code/neatcontext/src/core/routing.mjs +++ b/plugins/claude-code/neatcontext/src/core/routing.mjs @@ -98,6 +98,10 @@ const STICKY_BOOST = 1.35; const DECLINE_HALF_LIFE_DAYS = 14; const DECLINE_LIFETIME_DAYS = 42; + +// How long a refusal bars connecting unasked, as opposed to merely discounting +// the score. See `hasLiveDecline` for why the two differ. +const LIVE_DECLINE_DAYS = DECLINE_HALF_LIFE_DAYS; const DECLINE_WEIGHT = 0.4; const MAX_DECLINE_COUNT = 10; const DAY_MS = 24 * 60 * 60 * 1000; @@ -220,9 +224,45 @@ function capDecisions(decisions) { for (const decision of decisions) { (decision?.automatic === true ? automatic : manual).push(decision); } - return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( - (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) - ); + return mergeByTime(manual.slice(-MAX_DECISIONS), automatic.slice(-MAX_AUTOMATIC_DECISIONS)); +} + +// Where a decision sits in the log, for a decision whose `at` will not parse. +// +// It sits where it already was. `Date.parse` returns NaN for a hand-edited +// entry, a half-written one, or one from a future writer that spells the field +// differently, and `NaN || 0` reads that as the first of January 1970 — which +// sorts it to the front of the log and, because this runs on every write, +// leaves it there for good. One unreadable timestamp then permanently rewrites +// the chronology `familiarity` and every "why did it route that way?" read +// back off this file. +// +// Each bucket is already in the order it was appended, so carrying the last +// timestamp forward within it keeps such an entry beside the decisions it was +// actually made among — the only evidence about it that is left. +function timeKeys(bucket) { + let last = -Infinity; + return bucket.map((decision) => { + const parsed = Date.parse(decision?.at); + if (Number.isFinite(parsed)) { + last = parsed; + } + return last; + }); +} + +// A merge rather than a sort, because both sides arrive ordered and a merge is +// the one way to interleave them that cannot move anything within its own side. +function mergeByTime(left, right) { + const leftKeys = timeKeys(left); + const rightKeys = timeKeys(right); + const merged = []; + let l = 0; + let r = 0; + while (l < left.length && r < right.length) { + merged.push(leftKeys[l] <= rightKeys[r] ? left[l++] : right[r++]); + } + return [...merged, ...left.slice(l), ...right.slice(r)]; } async function update(mutate) { @@ -395,6 +435,30 @@ export function declineFactor(state, contextId, now = new Date()) { return (1 - strength) ** entry.count; } +// Whether a refusal is recent enough to forbid connecting without being asked. +// +// Deliberately not `declineFactor(...) < 1`. That reads a veto off a floating +// point comparison, and it is true for the whole six weeks the multiplier takes +// to decay: at day 41 the factor is about 0.99, a discount the ranking treats +// as no discount at all, while the gate was treating it as an absolute bar. It +// also leaves the threshold — the thing a reader most needs to know — implicit +// in a value nothing names. +// +// One half-life is where the line goes. For that long a refusal still carries +// most of the weight it was given, and connecting unasked is the one route the +// user gets no chance to stop. Past it, the multiplier is the whole answer: a +// faded refusal is a hint, and the ranking is where hints belong. +// +// A timestamp from the future counts as live, matching how `declineFactor` +// floors the age at zero. Both err towards not acting. +export function hasLiveDecline(state, contextId, now = new Date()) { + const at = Date.parse(state?.declines?.[contextId]?.at); + if (!Number.isFinite(at)) { + return false; + } + return (now.getTime() - at) / DAY_MS < LIVE_DECLINE_DAYS; +} + // How much this machine's own history argues for a context: a multiplier at or // above 1, never below, because this is a hint and not evidence. // @@ -456,7 +520,22 @@ export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); const id = entry.sessionId; - if (id) { + // A session record holds what the user chose for that window: its `mode` + // override and the contexts they declined in it. Every writer of one used + // to need a person or a model to act — `setMode`, `noteDeclined`, or a + // `use_context` call. An automatic route needs neither, and it arrives at + // about one per new session, so creating a record for one turned the + // `MAX_SESSIONS` cap into a shredder: twenty windows auto-connecting + // elsewhere would evict the record of a window where somebody had run + // `/neatcontext:mode manual`, `resolveMode` would fall through to the + // default — which is `auto` — and a session where the user had turned + // routing off would start routing itself again, silently, having also + // forgotten what they declined there. + // + // So an automatic decision keeps a record that already exists up to date, + // and creates none. The log still has the route: `decisions` is where a + // machine route belongs, and it is capped in its own bucket. + if (id && (state.sessions[id] || entry.automatic !== true)) { const session = state.sessions[id] ?? {}; state.sessions[id] = { ...session, diff --git a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs index 6314d1a..2afd881 100644 --- a/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs +++ b/plugins/copilot/neatcontext/src/copilot/mcp-bridge.mjs @@ -23,7 +23,7 @@ import { parseQualifiedToolName } from "../core/extensions.mjs"; import { addAlias, DEFAULT_MODE, - declineFactor, + hasLiveDecline, menuEntries, noteDecision, noteDeclined, @@ -51,8 +51,8 @@ const GET_CONTEXT_TOOL = { "user's own domain, documents, tools, or team conventions — some hosts do not surface " + "this server's initialize instructions, so the tool description is what carries that rule. " + "Pass the user's request as query before calling use_context: when nothing is connected, " + - "this tool safely auto-connects a uniquely clear match in auto mode or returns the routing " + - "menu needed to choose, ask, or decline.", + "this tool can safely auto-connect a uniquely clear match in auto mode, and otherwise " + + "returns the routing menu needed to choose, ask, or decline.", inputSchema: { type: "object", properties: { @@ -104,11 +104,16 @@ const NOTHING_CONNECTED = // when nothing was ever compared. That is the same rule the comment above sets // out for the handshake instructions: text that cannot know the current state // must not assert it. -function nothingConnectedRoutable(assessed) { +function nothingConnectedRoutable(assessed, shared) { return ( `${NOTHING_CONNECTED_HEAD} There are contexts on this machine, listed below with what each ` + "one is for. " + (assessed ? "No safe automatic match was made for this call. " : "") + + (shared + ? "Automatic connection is off in this window: this host gives it no session of its own, " + + "so connecting one here would change what every other window open on this folder is " + + "grounded in. Connect it yourself with `use_context`. " + : "") + "Follow the routing rules below: connect a clear choice with `use_context`, ask when the " + "choice is ambiguous, or say none covers the request. Do not ask the user to run a command " + "to connect a context you can already name. If none covers the request, offer " + @@ -252,7 +257,9 @@ function nothingConnectedText(pass) { if (pass.mode === "manual") { return NOTHING_CONNECTED; } - return pass.mode === "ask" ? NOTHING_CONNECTED_ASK : nothingConnectedRoutable(pass.assessed); + return pass.mode === "ask" + ? NOTHING_CONNECTED_ASK + : nothingConnectedRoutable(pass.assessed, pass.shared); } // The selected context, or null when nothing is selected. A selection @@ -382,8 +389,21 @@ function routingMenu(pass) { // SHORTLIST_MIN_CONTEXTS never builds a shortlist at all — so reading it off // the shortlist alone left the full menu, the one place the model has least // to go on, as the only caller that never heard about it. + // + // Assessed again over the shortlist, though, because the tie note names its + // leaders and the model is asked to say what each one covers. `pass.decision` + // is over the whole corpus — right for the gate, which needs the full field + // to know a leader is uncontested, and wrong here: with eight contexts inside + // the ratio band it named all eight, three of them absent from the list + // printed directly above, and asked the model to describe contexts it had + // never been shown. The verdict itself cannot differ — the shortlist is the + // top of the same ranking, so a leader uncontested in the corpus is + // uncontested in its prefix — only the names it carries. + // + // `renderMenu` needs no such trim: it is reached only below + // SHORTLIST_MIN_CONTEXTS, where it prints every context there is. return shortlist - ? renderShortlist(shortlist, { ...options, decision: pass.decision }) + ? renderShortlist(shortlist, { ...options, decision: assess(shortlist) }) : renderMenu(entries, { ...options, decision: pass.decision }); } @@ -423,7 +443,8 @@ async function routingPass(query) { ranked: null, decision: null, connected: null, - assessed: false + assessed: false, + shared: false }; // Everything is inside one guard, starting with `sessionId()` — it reads @@ -455,6 +476,16 @@ async function routingPass(query) { return pass; } + // Manual is the mode in which the plugin never routes, and both renderers + // return null for it — `renderMenu` and `renderShortlist` alike — so no + // reader for a ranking made here exists. Producing one anyway cost a + // knowledge-folder listing per context, BM25 over the corpus, and a decline + // lookup and decision-log walk per candidate, on every queried call, to + // build a list thrown away on the next line. + if (pass.mode === "manual") { + return pass; + } + // Every candidate, not a top slice: the tie check is only as good as the // field it can see. The shortlist takes its own slice of this afterwards // rather than ranking the corpus a second time. @@ -467,7 +498,21 @@ async function routingPass(query) { // From here it is about acting unasked. `assessed` stays a statement about // that specific question, so the nothing-connected text only claims a match // was looked for when one actually was. - if (pass.connectedId || pass.mode !== "auto" || !hasHostSessionId()) { + if (pass.connectedId || pass.mode !== "auto") { + return pass; + } + + // Auto mode with a request to match and nothing connected — everything the + // feature needs except a window it can call its own. Without a host session + // id, `sessionId()` is the workspace digest every window on this folder + // shares, and connecting on that re-grounds the conversation running next + // door. Recorded rather than just returned: `get_context`'s own description + // tells the model this call can connect a clear match, and sharing an early + // return with the mode and connected checks left the one case where that is + // never true saying nothing at all. Silence there reads as "nothing + // matched", which is a claim about the store rather than about the host. + if (!hasHostSessionId()) { + pass.shared = true; return pass; } pass.assessed = true; @@ -494,7 +539,7 @@ async function routingPass(query) { // route went through the model, because calling `use_context` announces the // switch and gives the user somewhere to say no again. Acting unasked // removes the announcement, so a live refusal disqualifies it outright. - if (declineFactor(state, target.id) < 1) { + if (hasLiveDecline(state, target.id)) { return pass; } diff --git a/plugins/copilot/neatcontext/src/core/routing-candidates.mjs b/plugins/copilot/neatcontext/src/core/routing-candidates.mjs index a6455c8..82c1751 100644 --- a/plugins/copilot/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/copilot/neatcontext/src/core/routing-candidates.mjs @@ -178,6 +178,40 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } + + // A compound and the parts `tokenize` derived from it are one thing this + // context knows about, and they have to arrive on this side as one. + // + // The pairing below is independent on both sides, but `matched` is not: + // `rank` returns every token the index holds, and a description containing + // `user_id` indexes `user_id`, `user` and `id`. All three come back, so a + // request that named the one concept twice found two distinct things waiting + // to be paired with — "what does user_id mean for a user?" cleared a floor of + // two on `user_id` and `user`, which is the very bypass this floor exists to + // close, arriving on the other side of it. + // + // The longest spelling is what is kept, and only its own derived parts are + // dropped: `user` and `users` derive from neither each other nor a common + // compound, so two things that really are two still count as two. Deletion is + // by derivation rather than by substring for the same reason — `id` inside + // `identity` is a different word, and dropping it would silently disarm the + // floor for any context whose description happens to contain a longer word. + // + // One direction is given up deliberately. A description that carries a + // derived part as a word of its own — "the `user_id` in the `user` table" — + // is indistinguishable here from one that only carries the compound, and both + // collapse to one. That costs an auto-connection on a description that really + // did name two things; the alternative costs a session re-grounded on a + // context it only half matched, unannounced. On the one surface that acts + // without asking, the miss is the cheaper mistake. + for (const term of [...carried]) { + for (const part of tokenize(term)) { + if (part !== term) { + carried.delete(part); + } + } + } + const options = []; for (const term of queryTerms(query)) { const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); diff --git a/plugins/copilot/neatcontext/src/core/routing.mjs b/plugins/copilot/neatcontext/src/core/routing.mjs index 266e040..a7a2931 100644 --- a/plugins/copilot/neatcontext/src/core/routing.mjs +++ b/plugins/copilot/neatcontext/src/core/routing.mjs @@ -98,6 +98,10 @@ const STICKY_BOOST = 1.35; const DECLINE_HALF_LIFE_DAYS = 14; const DECLINE_LIFETIME_DAYS = 42; + +// How long a refusal bars connecting unasked, as opposed to merely discounting +// the score. See `hasLiveDecline` for why the two differ. +const LIVE_DECLINE_DAYS = DECLINE_HALF_LIFE_DAYS; const DECLINE_WEIGHT = 0.4; const MAX_DECLINE_COUNT = 10; const DAY_MS = 24 * 60 * 60 * 1000; @@ -220,9 +224,45 @@ function capDecisions(decisions) { for (const decision of decisions) { (decision?.automatic === true ? automatic : manual).push(decision); } - return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( - (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) - ); + return mergeByTime(manual.slice(-MAX_DECISIONS), automatic.slice(-MAX_AUTOMATIC_DECISIONS)); +} + +// Where a decision sits in the log, for a decision whose `at` will not parse. +// +// It sits where it already was. `Date.parse` returns NaN for a hand-edited +// entry, a half-written one, or one from a future writer that spells the field +// differently, and `NaN || 0` reads that as the first of January 1970 — which +// sorts it to the front of the log and, because this runs on every write, +// leaves it there for good. One unreadable timestamp then permanently rewrites +// the chronology `familiarity` and every "why did it route that way?" read +// back off this file. +// +// Each bucket is already in the order it was appended, so carrying the last +// timestamp forward within it keeps such an entry beside the decisions it was +// actually made among — the only evidence about it that is left. +function timeKeys(bucket) { + let last = -Infinity; + return bucket.map((decision) => { + const parsed = Date.parse(decision?.at); + if (Number.isFinite(parsed)) { + last = parsed; + } + return last; + }); +} + +// A merge rather than a sort, because both sides arrive ordered and a merge is +// the one way to interleave them that cannot move anything within its own side. +function mergeByTime(left, right) { + const leftKeys = timeKeys(left); + const rightKeys = timeKeys(right); + const merged = []; + let l = 0; + let r = 0; + while (l < left.length && r < right.length) { + merged.push(leftKeys[l] <= rightKeys[r] ? left[l++] : right[r++]); + } + return [...merged, ...left.slice(l), ...right.slice(r)]; } async function update(mutate) { @@ -395,6 +435,30 @@ export function declineFactor(state, contextId, now = new Date()) { return (1 - strength) ** entry.count; } +// Whether a refusal is recent enough to forbid connecting without being asked. +// +// Deliberately not `declineFactor(...) < 1`. That reads a veto off a floating +// point comparison, and it is true for the whole six weeks the multiplier takes +// to decay: at day 41 the factor is about 0.99, a discount the ranking treats +// as no discount at all, while the gate was treating it as an absolute bar. It +// also leaves the threshold — the thing a reader most needs to know — implicit +// in a value nothing names. +// +// One half-life is where the line goes. For that long a refusal still carries +// most of the weight it was given, and connecting unasked is the one route the +// user gets no chance to stop. Past it, the multiplier is the whole answer: a +// faded refusal is a hint, and the ranking is where hints belong. +// +// A timestamp from the future counts as live, matching how `declineFactor` +// floors the age at zero. Both err towards not acting. +export function hasLiveDecline(state, contextId, now = new Date()) { + const at = Date.parse(state?.declines?.[contextId]?.at); + if (!Number.isFinite(at)) { + return false; + } + return (now.getTime() - at) / DAY_MS < LIVE_DECLINE_DAYS; +} + // How much this machine's own history argues for a context: a multiplier at or // above 1, never below, because this is a hint and not evidence. // @@ -456,7 +520,22 @@ export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); const id = entry.sessionId; - if (id) { + // A session record holds what the user chose for that window: its `mode` + // override and the contexts they declined in it. Every writer of one used + // to need a person or a model to act — `setMode`, `noteDeclined`, or a + // `use_context` call. An automatic route needs neither, and it arrives at + // about one per new session, so creating a record for one turned the + // `MAX_SESSIONS` cap into a shredder: twenty windows auto-connecting + // elsewhere would evict the record of a window where somebody had run + // `/neatcontext:mode manual`, `resolveMode` would fall through to the + // default — which is `auto` — and a session where the user had turned + // routing off would start routing itself again, silently, having also + // forgotten what they declined there. + // + // So an automatic decision keeps a record that already exists up to date, + // and creates none. The log still has the route: `decisions` is where a + // machine route belongs, and it is capped in its own bucket. + if (id && (state.sessions[id] || entry.automatic !== true)) { const session = state.sessions[id] ?? {}; state.sessions[id] = { ...session, diff --git a/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs b/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs index a6455c8..82c1751 100644 --- a/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/kimi-code/neatcontext/src/core/routing-candidates.mjs @@ -178,6 +178,40 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } + + // A compound and the parts `tokenize` derived from it are one thing this + // context knows about, and they have to arrive on this side as one. + // + // The pairing below is independent on both sides, but `matched` is not: + // `rank` returns every token the index holds, and a description containing + // `user_id` indexes `user_id`, `user` and `id`. All three come back, so a + // request that named the one concept twice found two distinct things waiting + // to be paired with — "what does user_id mean for a user?" cleared a floor of + // two on `user_id` and `user`, which is the very bypass this floor exists to + // close, arriving on the other side of it. + // + // The longest spelling is what is kept, and only its own derived parts are + // dropped: `user` and `users` derive from neither each other nor a common + // compound, so two things that really are two still count as two. Deletion is + // by derivation rather than by substring for the same reason — `id` inside + // `identity` is a different word, and dropping it would silently disarm the + // floor for any context whose description happens to contain a longer word. + // + // One direction is given up deliberately. A description that carries a + // derived part as a word of its own — "the `user_id` in the `user` table" — + // is indistinguishable here from one that only carries the compound, and both + // collapse to one. That costs an auto-connection on a description that really + // did name two things; the alternative costs a session re-grounded on a + // context it only half matched, unannounced. On the one surface that acts + // without asking, the miss is the cheaper mistake. + for (const term of [...carried]) { + for (const part of tokenize(term)) { + if (part !== term) { + carried.delete(part); + } + } + } + const options = []; for (const term of queryTerms(query)) { const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); diff --git a/plugins/kimi-code/neatcontext/src/core/routing.mjs b/plugins/kimi-code/neatcontext/src/core/routing.mjs index 266e040..a7a2931 100644 --- a/plugins/kimi-code/neatcontext/src/core/routing.mjs +++ b/plugins/kimi-code/neatcontext/src/core/routing.mjs @@ -98,6 +98,10 @@ const STICKY_BOOST = 1.35; const DECLINE_HALF_LIFE_DAYS = 14; const DECLINE_LIFETIME_DAYS = 42; + +// How long a refusal bars connecting unasked, as opposed to merely discounting +// the score. See `hasLiveDecline` for why the two differ. +const LIVE_DECLINE_DAYS = DECLINE_HALF_LIFE_DAYS; const DECLINE_WEIGHT = 0.4; const MAX_DECLINE_COUNT = 10; const DAY_MS = 24 * 60 * 60 * 1000; @@ -220,9 +224,45 @@ function capDecisions(decisions) { for (const decision of decisions) { (decision?.automatic === true ? automatic : manual).push(decision); } - return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( - (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) - ); + return mergeByTime(manual.slice(-MAX_DECISIONS), automatic.slice(-MAX_AUTOMATIC_DECISIONS)); +} + +// Where a decision sits in the log, for a decision whose `at` will not parse. +// +// It sits where it already was. `Date.parse` returns NaN for a hand-edited +// entry, a half-written one, or one from a future writer that spells the field +// differently, and `NaN || 0` reads that as the first of January 1970 — which +// sorts it to the front of the log and, because this runs on every write, +// leaves it there for good. One unreadable timestamp then permanently rewrites +// the chronology `familiarity` and every "why did it route that way?" read +// back off this file. +// +// Each bucket is already in the order it was appended, so carrying the last +// timestamp forward within it keeps such an entry beside the decisions it was +// actually made among — the only evidence about it that is left. +function timeKeys(bucket) { + let last = -Infinity; + return bucket.map((decision) => { + const parsed = Date.parse(decision?.at); + if (Number.isFinite(parsed)) { + last = parsed; + } + return last; + }); +} + +// A merge rather than a sort, because both sides arrive ordered and a merge is +// the one way to interleave them that cannot move anything within its own side. +function mergeByTime(left, right) { + const leftKeys = timeKeys(left); + const rightKeys = timeKeys(right); + const merged = []; + let l = 0; + let r = 0; + while (l < left.length && r < right.length) { + merged.push(leftKeys[l] <= rightKeys[r] ? left[l++] : right[r++]); + } + return [...merged, ...left.slice(l), ...right.slice(r)]; } async function update(mutate) { @@ -395,6 +435,30 @@ export function declineFactor(state, contextId, now = new Date()) { return (1 - strength) ** entry.count; } +// Whether a refusal is recent enough to forbid connecting without being asked. +// +// Deliberately not `declineFactor(...) < 1`. That reads a veto off a floating +// point comparison, and it is true for the whole six weeks the multiplier takes +// to decay: at day 41 the factor is about 0.99, a discount the ranking treats +// as no discount at all, while the gate was treating it as an absolute bar. It +// also leaves the threshold — the thing a reader most needs to know — implicit +// in a value nothing names. +// +// One half-life is where the line goes. For that long a refusal still carries +// most of the weight it was given, and connecting unasked is the one route the +// user gets no chance to stop. Past it, the multiplier is the whole answer: a +// faded refusal is a hint, and the ranking is where hints belong. +// +// A timestamp from the future counts as live, matching how `declineFactor` +// floors the age at zero. Both err towards not acting. +export function hasLiveDecline(state, contextId, now = new Date()) { + const at = Date.parse(state?.declines?.[contextId]?.at); + if (!Number.isFinite(at)) { + return false; + } + return (now.getTime() - at) / DAY_MS < LIVE_DECLINE_DAYS; +} + // How much this machine's own history argues for a context: a multiplier at or // above 1, never below, because this is a hint and not evidence. // @@ -456,7 +520,22 @@ export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); const id = entry.sessionId; - if (id) { + // A session record holds what the user chose for that window: its `mode` + // override and the contexts they declined in it. Every writer of one used + // to need a person or a model to act — `setMode`, `noteDeclined`, or a + // `use_context` call. An automatic route needs neither, and it arrives at + // about one per new session, so creating a record for one turned the + // `MAX_SESSIONS` cap into a shredder: twenty windows auto-connecting + // elsewhere would evict the record of a window where somebody had run + // `/neatcontext:mode manual`, `resolveMode` would fall through to the + // default — which is `auto` — and a session where the user had turned + // routing off would start routing itself again, silently, having also + // forgotten what they declined there. + // + // So an automatic decision keeps a record that already exists up to date, + // and creates none. The log still has the route: `decisions` is where a + // machine route belongs, and it is capped in its own bucket. + if (id && (state.sessions[id] || entry.automatic !== true)) { const session = state.sessions[id] ?? {}; state.sessions[id] = { ...session, diff --git a/plugins/pi/neatcontext/src/core/routing-candidates.mjs b/plugins/pi/neatcontext/src/core/routing-candidates.mjs index a6455c8..82c1751 100644 --- a/plugins/pi/neatcontext/src/core/routing-candidates.mjs +++ b/plugins/pi/neatcontext/src/core/routing-candidates.mjs @@ -178,6 +178,40 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } + + // A compound and the parts `tokenize` derived from it are one thing this + // context knows about, and they have to arrive on this side as one. + // + // The pairing below is independent on both sides, but `matched` is not: + // `rank` returns every token the index holds, and a description containing + // `user_id` indexes `user_id`, `user` and `id`. All three come back, so a + // request that named the one concept twice found two distinct things waiting + // to be paired with — "what does user_id mean for a user?" cleared a floor of + // two on `user_id` and `user`, which is the very bypass this floor exists to + // close, arriving on the other side of it. + // + // The longest spelling is what is kept, and only its own derived parts are + // dropped: `user` and `users` derive from neither each other nor a common + // compound, so two things that really are two still count as two. Deletion is + // by derivation rather than by substring for the same reason — `id` inside + // `identity` is a different word, and dropping it would silently disarm the + // floor for any context whose description happens to contain a longer word. + // + // One direction is given up deliberately. A description that carries a + // derived part as a word of its own — "the `user_id` in the `user` table" — + // is indistinguishable here from one that only carries the compound, and both + // collapse to one. That costs an auto-connection on a description that really + // did name two things; the alternative costs a session re-grounded on a + // context it only half matched, unannounced. On the one surface that acts + // without asking, the miss is the cheaper mistake. + for (const term of [...carried]) { + for (const part of tokenize(term)) { + if (part !== term) { + carried.delete(part); + } + } + } + const options = []; for (const term of queryTerms(query)) { const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); diff --git a/plugins/pi/neatcontext/src/core/routing.mjs b/plugins/pi/neatcontext/src/core/routing.mjs index 266e040..a7a2931 100644 --- a/plugins/pi/neatcontext/src/core/routing.mjs +++ b/plugins/pi/neatcontext/src/core/routing.mjs @@ -98,6 +98,10 @@ const STICKY_BOOST = 1.35; const DECLINE_HALF_LIFE_DAYS = 14; const DECLINE_LIFETIME_DAYS = 42; + +// How long a refusal bars connecting unasked, as opposed to merely discounting +// the score. See `hasLiveDecline` for why the two differ. +const LIVE_DECLINE_DAYS = DECLINE_HALF_LIFE_DAYS; const DECLINE_WEIGHT = 0.4; const MAX_DECLINE_COUNT = 10; const DAY_MS = 24 * 60 * 60 * 1000; @@ -220,9 +224,45 @@ function capDecisions(decisions) { for (const decision of decisions) { (decision?.automatic === true ? automatic : manual).push(decision); } - return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( - (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) - ); + return mergeByTime(manual.slice(-MAX_DECISIONS), automatic.slice(-MAX_AUTOMATIC_DECISIONS)); +} + +// Where a decision sits in the log, for a decision whose `at` will not parse. +// +// It sits where it already was. `Date.parse` returns NaN for a hand-edited +// entry, a half-written one, or one from a future writer that spells the field +// differently, and `NaN || 0` reads that as the first of January 1970 — which +// sorts it to the front of the log and, because this runs on every write, +// leaves it there for good. One unreadable timestamp then permanently rewrites +// the chronology `familiarity` and every "why did it route that way?" read +// back off this file. +// +// Each bucket is already in the order it was appended, so carrying the last +// timestamp forward within it keeps such an entry beside the decisions it was +// actually made among — the only evidence about it that is left. +function timeKeys(bucket) { + let last = -Infinity; + return bucket.map((decision) => { + const parsed = Date.parse(decision?.at); + if (Number.isFinite(parsed)) { + last = parsed; + } + return last; + }); +} + +// A merge rather than a sort, because both sides arrive ordered and a merge is +// the one way to interleave them that cannot move anything within its own side. +function mergeByTime(left, right) { + const leftKeys = timeKeys(left); + const rightKeys = timeKeys(right); + const merged = []; + let l = 0; + let r = 0; + while (l < left.length && r < right.length) { + merged.push(leftKeys[l] <= rightKeys[r] ? left[l++] : right[r++]); + } + return [...merged, ...left.slice(l), ...right.slice(r)]; } async function update(mutate) { @@ -395,6 +435,30 @@ export function declineFactor(state, contextId, now = new Date()) { return (1 - strength) ** entry.count; } +// Whether a refusal is recent enough to forbid connecting without being asked. +// +// Deliberately not `declineFactor(...) < 1`. That reads a veto off a floating +// point comparison, and it is true for the whole six weeks the multiplier takes +// to decay: at day 41 the factor is about 0.99, a discount the ranking treats +// as no discount at all, while the gate was treating it as an absolute bar. It +// also leaves the threshold — the thing a reader most needs to know — implicit +// in a value nothing names. +// +// One half-life is where the line goes. For that long a refusal still carries +// most of the weight it was given, and connecting unasked is the one route the +// user gets no chance to stop. Past it, the multiplier is the whole answer: a +// faded refusal is a hint, and the ranking is where hints belong. +// +// A timestamp from the future counts as live, matching how `declineFactor` +// floors the age at zero. Both err towards not acting. +export function hasLiveDecline(state, contextId, now = new Date()) { + const at = Date.parse(state?.declines?.[contextId]?.at); + if (!Number.isFinite(at)) { + return false; + } + return (now.getTime() - at) / DAY_MS < LIVE_DECLINE_DAYS; +} + // How much this machine's own history argues for a context: a multiplier at or // above 1, never below, because this is a hint and not evidence. // @@ -456,7 +520,22 @@ export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); const id = entry.sessionId; - if (id) { + // A session record holds what the user chose for that window: its `mode` + // override and the contexts they declined in it. Every writer of one used + // to need a person or a model to act — `setMode`, `noteDeclined`, or a + // `use_context` call. An automatic route needs neither, and it arrives at + // about one per new session, so creating a record for one turned the + // `MAX_SESSIONS` cap into a shredder: twenty windows auto-connecting + // elsewhere would evict the record of a window where somebody had run + // `/neatcontext:mode manual`, `resolveMode` would fall through to the + // default — which is `auto` — and a session where the user had turned + // routing off would start routing itself again, silently, having also + // forgotten what they declined there. + // + // So an automatic decision keeps a record that already exists up to date, + // and creates none. The log still has the route: `decisions` is where a + // machine route belongs, and it is capped in its own bucket. + if (id && (state.sessions[id] || entry.automatic !== true)) { const session = state.sessions[id] ?? {}; state.sessions[id] = { ...session, diff --git a/shared/core/routing-candidates.mjs b/shared/core/routing-candidates.mjs index a6455c8..82c1751 100644 --- a/shared/core/routing-candidates.mjs +++ b/shared/core/routing-candidates.mjs @@ -178,6 +178,40 @@ function agreeingTerms(candidate, query) { if (carried.size === 0) { return 0; } + + // A compound and the parts `tokenize` derived from it are one thing this + // context knows about, and they have to arrive on this side as one. + // + // The pairing below is independent on both sides, but `matched` is not: + // `rank` returns every token the index holds, and a description containing + // `user_id` indexes `user_id`, `user` and `id`. All three come back, so a + // request that named the one concept twice found two distinct things waiting + // to be paired with — "what does user_id mean for a user?" cleared a floor of + // two on `user_id` and `user`, which is the very bypass this floor exists to + // close, arriving on the other side of it. + // + // The longest spelling is what is kept, and only its own derived parts are + // dropped: `user` and `users` derive from neither each other nor a common + // compound, so two things that really are two still count as two. Deletion is + // by derivation rather than by substring for the same reason — `id` inside + // `identity` is a different word, and dropping it would silently disarm the + // floor for any context whose description happens to contain a longer word. + // + // One direction is given up deliberately. A description that carries a + // derived part as a word of its own — "the `user_id` in the `user` table" — + // is indistinguishable here from one that only carries the compound, and both + // collapse to one. That costs an auto-connection on a description that really + // did name two things; the alternative costs a session re-grounded on a + // context it only half matched, unannounced. On the one surface that acts + // without asking, the miss is the cheaper mistake. + for (const term of [...carried]) { + for (const part of tokenize(term)) { + if (part !== term) { + carried.delete(part); + } + } + } + const options = []; for (const term of queryTerms(query)) { const agreed = [...new Set(tokenize(term))].filter((token) => carried.has(token)); diff --git a/shared/core/routing.mjs b/shared/core/routing.mjs index 266e040..a7a2931 100644 --- a/shared/core/routing.mjs +++ b/shared/core/routing.mjs @@ -98,6 +98,10 @@ const STICKY_BOOST = 1.35; const DECLINE_HALF_LIFE_DAYS = 14; const DECLINE_LIFETIME_DAYS = 42; + +// How long a refusal bars connecting unasked, as opposed to merely discounting +// the score. See `hasLiveDecline` for why the two differ. +const LIVE_DECLINE_DAYS = DECLINE_HALF_LIFE_DAYS; const DECLINE_WEIGHT = 0.4; const MAX_DECLINE_COUNT = 10; const DAY_MS = 24 * 60 * 60 * 1000; @@ -220,9 +224,45 @@ function capDecisions(decisions) { for (const decision of decisions) { (decision?.automatic === true ? automatic : manual).push(decision); } - return [...manual.slice(-MAX_DECISIONS), ...automatic.slice(-MAX_AUTOMATIC_DECISIONS)].sort( - (left, right) => (Date.parse(left?.at) || 0) - (Date.parse(right?.at) || 0) - ); + return mergeByTime(manual.slice(-MAX_DECISIONS), automatic.slice(-MAX_AUTOMATIC_DECISIONS)); +} + +// Where a decision sits in the log, for a decision whose `at` will not parse. +// +// It sits where it already was. `Date.parse` returns NaN for a hand-edited +// entry, a half-written one, or one from a future writer that spells the field +// differently, and `NaN || 0` reads that as the first of January 1970 — which +// sorts it to the front of the log and, because this runs on every write, +// leaves it there for good. One unreadable timestamp then permanently rewrites +// the chronology `familiarity` and every "why did it route that way?" read +// back off this file. +// +// Each bucket is already in the order it was appended, so carrying the last +// timestamp forward within it keeps such an entry beside the decisions it was +// actually made among — the only evidence about it that is left. +function timeKeys(bucket) { + let last = -Infinity; + return bucket.map((decision) => { + const parsed = Date.parse(decision?.at); + if (Number.isFinite(parsed)) { + last = parsed; + } + return last; + }); +} + +// A merge rather than a sort, because both sides arrive ordered and a merge is +// the one way to interleave them that cannot move anything within its own side. +function mergeByTime(left, right) { + const leftKeys = timeKeys(left); + const rightKeys = timeKeys(right); + const merged = []; + let l = 0; + let r = 0; + while (l < left.length && r < right.length) { + merged.push(leftKeys[l] <= rightKeys[r] ? left[l++] : right[r++]); + } + return [...merged, ...left.slice(l), ...right.slice(r)]; } async function update(mutate) { @@ -395,6 +435,30 @@ export function declineFactor(state, contextId, now = new Date()) { return (1 - strength) ** entry.count; } +// Whether a refusal is recent enough to forbid connecting without being asked. +// +// Deliberately not `declineFactor(...) < 1`. That reads a veto off a floating +// point comparison, and it is true for the whole six weeks the multiplier takes +// to decay: at day 41 the factor is about 0.99, a discount the ranking treats +// as no discount at all, while the gate was treating it as an absolute bar. It +// also leaves the threshold — the thing a reader most needs to know — implicit +// in a value nothing names. +// +// One half-life is where the line goes. For that long a refusal still carries +// most of the weight it was given, and connecting unasked is the one route the +// user gets no chance to stop. Past it, the multiplier is the whole answer: a +// faded refusal is a hint, and the ranking is where hints belong. +// +// A timestamp from the future counts as live, matching how `declineFactor` +// floors the age at zero. Both err towards not acting. +export function hasLiveDecline(state, contextId, now = new Date()) { + const at = Date.parse(state?.declines?.[contextId]?.at); + if (!Number.isFinite(at)) { + return false; + } + return (now.getTime() - at) / DAY_MS < LIVE_DECLINE_DAYS; +} + // How much this machine's own history argues for a context: a multiplier at or // above 1, never below, because this is a hint and not evidence. // @@ -456,7 +520,22 @@ export function noteDecision(entry) { return update((state) => { state.decisions.push({ at: new Date().toISOString(), ...entry }); const id = entry.sessionId; - if (id) { + // A session record holds what the user chose for that window: its `mode` + // override and the contexts they declined in it. Every writer of one used + // to need a person or a model to act — `setMode`, `noteDeclined`, or a + // `use_context` call. An automatic route needs neither, and it arrives at + // about one per new session, so creating a record for one turned the + // `MAX_SESSIONS` cap into a shredder: twenty windows auto-connecting + // elsewhere would evict the record of a window where somebody had run + // `/neatcontext:mode manual`, `resolveMode` would fall through to the + // default — which is `auto` — and a session where the user had turned + // routing off would start routing itself again, silently, having also + // forgotten what they declined there. + // + // So an automatic decision keeps a record that already exists up to date, + // and creates none. The log still has the route: `decisions` is where a + // machine route belongs, and it is capped in its own bucket. + if (id && (state.sessions[id] || entry.automatic !== true)) { const session = state.sessions[id] ?? {}; state.sessions[id] = { ...session, diff --git a/tests/copilot-plugin.test.mjs b/tests/copilot-plugin.test.mjs index f789324..089dd11 100644 --- a/tests/copilot-plugin.test.mjs +++ b/tests/copilot-plugin.test.mjs @@ -8,7 +8,7 @@ import assert from "node:assert/strict"; import { spawn } from "node:child_process"; -import { chmod, mkdir, mkdtemp, readFile, readdir, rm, stat, writeFile } from "node:fs/promises"; +import { mkdir, mkdtemp, readFile, readdir, rm, stat, writeFile } from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import test from "node:test"; @@ -1171,6 +1171,12 @@ test("Copilot get_context does not auto-connect on evidence weaker than it looks // Auto-connect is the one route nobody announces before it happens, so it needs // a session it cannot leak out of. Without an id from the host, one selection // file is shared by every window open on the same folder. +// +// And it has to say so. `get_context`'s description tells the model this call +// can connect a clear match; on a host that publishes no session id it never +// will, and text that goes quiet about it reads as "nothing in the store +// matched" — a claim about the user's contexts made to excuse a limitation of +// their editor. test("Copilot get_context does not auto-connect without a host session id", async (t) => { const home = await isolatedHome("neatcontext-copilot-auto-connect-shared-"); const sessions = []; @@ -1190,8 +1196,17 @@ test("Copilot get_context does not auto-connect without a host session id", asyn const response = await session.call( toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) ); - assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); - assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + const text = response.result.content[0].text; + assert.doesNotMatch(text, /Automatically connected/); + assert.match(text, /No NeatContext Context is connected/); + assert.match(text, /Automatic connection is off in this window/); + assert.match(text, /gives it no session of its own/); + // The store was never the reason, so it must not be given as one. + assert.doesNotMatch(text, /No safe automatic match was made/); + // Routing itself is untouched — the model still gets the menu and the + // instruction to connect one by hand. + assert.match(text, /Checkout incident/); + assert.match(text, /connect a clear choice with `use_context`/); }); // A home it cannot write to must cost the caller an optimization, never the @@ -1409,6 +1424,120 @@ test("Copilot get_context does not auto-connect a context declined in another se assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); }); +// A refusal stops being a veto before it stops being a discount. +// +// `declineFactor` decays over six weeks and returns exactly 1 only at the end +// of them, so reading "not exactly 1" as a bar gave a refusal made a month and +// a half ago the same absolute force as one made this morning — at day 41 the +// multiplier is about 0.99, a number the ranking treats as no discount at all. +test("Copilot get_context auto-connects again once a refusal has aged out", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-declined-aged-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + await createContext(home, "Checkout incident", { + sessionId: "monday", + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + + const monday = rpcSession({ ...home.env, NEATCONTEXT_SESSION_ID: "monday" }); + sessions.push(monday); + await monday.call(initialize(1)); + const declined = await monday.call( + toolCall(2, "use_context", { context: "Checkout incident", declined: true }) + ); + assert.equal(declined.result.isError, false); + + // Backdated past the window in which a refusal forbids connecting unasked, + // but well inside the six weeks it takes the score discount to expire — the + // gap between the two is exactly what this pins. + const routingFile = path.join(home.directory, "plugin-routing.json"); + const routing = JSON.parse(await readFile(routingFile, "utf8")); + const declinedIds = Object.keys(routing.declines ?? {}); + assert.equal(declinedIds.length, 1); + const aged = new Date(Date.now() - 20 * 24 * 60 * 60 * 1000).toISOString(); + routing.declines[declinedIds[0]].at = aged; + await writeFile(routingFile, `${JSON.stringify(routing, null, 2)}\n`, "utf8"); + + const wednesday = rpcSession({ ...home.env, NEATCONTEXT_SESSION_ID: "wednesday" }); + sessions.push(wednesday); + await wednesday.call(initialize(1)); + const response = await wednesday.call( + toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + assert.match(response.result.content[0].text, /Automatically connected/); + assert.match(response.result.content[0].text, /Checkout incident/); +}); + +// A concept the user spelled twice is one piece of evidence, and the index is +// where that used to come apart: a description containing `user_id` indexes +// `user_id`, `user` and `id`, so all three come back as matched terms and a +// request naming the one concept twice found two separate things to agree with. +// Fixtures written by hand could not catch it — they were the one shape `rank` +// never produces. +test("Copilot get_context does not auto-connect on one concept spelled twice", async (t) => { + const home = await isolatedHome("neatcontext-copilot-auto-connect-one-concept-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-one-concept"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Identity", { + sessionId, + useWhen: "How the user_id column is populated." + }); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + const response = await session.call( + toolCall(2, "get_context", { query: "what does user_id mean for a user?" }) + ); + assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); + assert.match(response.result.content[0].text, /No NeatContext Context is connected/); + + // Two things genuinely agreed on still route, so the floor did not simply + // move out of reach. + const second = await session.call( + toolCall(3, "get_context", { query: "user_id column populated" }) + ); + assert.match(second.result.content[0].text, /Automatically connected/); +}); + +// Manual is the mode in which the plugin never routes, so the ranking a queried +// call used to build had no reader — both renderers return null for it. This +// pins that skipping the work changed none of the answers. +test("Copilot get_context answers manual mode the same way with a query", async (t) => { + const home = await isolatedHome("neatcontext-copilot-manual-query-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-manual-query"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + await createContext(home, "Checkout incident", { + sessionId, + useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" + }); + const mode = await runNode(cli, ["mode", "manual"], { env }); + assert.match(mode.stdout, /now manual for this session/); + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + const withQuery = await session.call( + toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + ); + const withoutQuery = await session.call(toolCall(3, "get_context")); + assert.equal(withQuery.result.content[0].text, withoutQuery.result.content[0].text); + assert.doesNotMatch(withQuery.result.content[0].text, /Automatically connected/); + assert.match(withQuery.result.content[0].text, /No NeatContext Context is connected/); + // No menu and no near-tie note: manual is the mode that does not route. + assert.doesNotMatch(withQuery.result.content[0].text, /Checkout incident/); +}); + // The near-tie is a property of the ranking, not of whether this particular // call was allowed to act on it. Read off the auto-connect path alone, the note // went silent in every situation that path bails out of — which is most of @@ -1444,6 +1573,48 @@ test("Copilot names a near-tie even when auto-connect was never on the table", a assert.doesNotMatch(response.result.content[0].text, /Automatically connected/); }); +// The note names the contexts it wants the model to describe, so it may only +// name contexts the model was shown. Assessed over the whole corpus — right for +// the auto-connect gate, which needs the full field to know a leader is +// uncontested — it named every context in the ratio band, three of them absent +// from the shortlist printed directly above it. +test("Copilot names only shortlisted contexts in the near-tie note", async (t) => { + const home = await isolatedHome("neatcontext-copilot-tie-shortlist-"); + const sessions = []; + t.after(async () => { + await Promise.all(sessions.map((session) => session.close())); + }); + const sessionId = "copilot-tie-shortlist"; + const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; + // Eight is where the shortlist starts, and one description between them puts + // every one of them inside the band. + const names = []; + for (let index = 0; index < 8; index += 1) { + const name = `Team ${index}`; + names.push(name); + await createContext(home, name, { + sessionId, + useWhen: "plugin packaging and marketplace manifests" + }); + } + + const session = rpcSession(env); + sessions.push(session); + await session.call(initialize(1)); + const response = await session.call( + toolCall(2, "get_context", { query: "plugin packaging and marketplace manifests" }) + ); + const text = response.result.content[0].text; + assert.match(text, /match the request about equally well/); + + const note = text.split("\n").find((line) => line.includes("match the request about equally")); + const named = names.filter((name) => note.includes(`**${name}**`)); + const listed = names.filter((name) => text.includes(`- **${name}**`)); + assert.equal(listed.length, 5, "the shortlist shows five"); + assert.deepEqual(named, listed, "the note names those five and no others"); + assert.doesNotMatch(text, /Automatically connected/); +}); + // A selection pointing at a context that is gone is nothing connected — // `readSelection` deletes the file on its way out. Read as connected, the one // call the user most needs a straight answer on says nothing is connected while @@ -1491,24 +1662,33 @@ test("Copilot reports the connection it made even when the decision log cannot b }); const sessionId = "copilot-auto-connect-log-fails"; const env = { ...home.env, NEATCONTEXT_SESSION_ID: sessionId }; - await createContext(home, "Checkout incident", { + // Matchable by name, which lives in the context's own manifest. The routing + // description would not survive what happens next: `--use-when` is stored as + // a card inside the very file this test is about to make unwritable, so a + // context relying on one stops matching at the same moment writes start + // failing, and the test would pass without the path under test ever running. + await createContext(home, "Pgbouncer exhaustion", { sessionId, useWhen: "checkout-api 5xx from pgbouncer pool exhaustion" }); - // Read-only where the routing state file goes: it still reads, so the card - // that makes this context matchable survives, but every write to it fails — - // which is what a hostile permission set looks like from here. + // A directory where the routing state file goes. Writes to it fail whoever is + // running — `chmod 0o444` denies nothing to root, which is the default user in + // most Docker CI images, and on Windows sets a read-only attribute Node's + // `writeFile` is not obliged to honour. Reads fail too, so the card written at + // creation is gone; the name is what the match is left with, which is the + // point. The unwritable-selection test above forces the same failure from the + // other side, writing a file where a directory is expected. const routingFile = path.join(home.directory, "plugin-routing.json"); - await chmod(routingFile, 0o444); - t.after(() => chmod(routingFile, 0o644).catch(() => undefined)); + await rm(routingFile, { force: true }); + await mkdir(routingFile, { recursive: true }); const session = rpcSession(env); sessions.push(session); await session.call(initialize(1)); const response = await session.call( - toolCall(2, "get_context", { query: "checkout-api 5xx pgbouncer pool exhaustion" }) + toolCall(2, "get_context", { query: "pgbouncer pool exhaustion" }) ); - assert.match(response.result.content[0].text, /Checkout incident/); + assert.match(response.result.content[0].text, /Pgbouncer exhaustion/); assert.doesNotMatch(response.result.content[0].text, /No NeatContext Context is connected/); }); diff --git a/tests/routing-confidence.test.mjs b/tests/routing-confidence.test.mjs index 2cd441d..dc3480e 100644 --- a/tests/routing-confidence.test.mjs +++ b/tests/routing-confidence.test.mjs @@ -97,6 +97,27 @@ function bridge(sessionId = "confidence-bridge") { const candidate = (id, score) => ({ id, name: id, score }); +const { buildIndex, rank } = await import( + "../plugins/claude-code/neatcontext/src/core/routing-search.mjs" +); +const { routingFields } = await import( + "../plugins/claude-code/neatcontext/src/core/routing-candidates.mjs" +); + +// The floor as the bridge actually meets it: `matched` filled by `rank` from a +// real index, rather than written by hand. +// +// Every hand-written fixture is a claim about what the index would return, and +// a wrong one hides the bug it was meant to catch — a leader carrying `user` +// but not `user_id` is a shape `rank` never produces, and the guard against +// one concept spelled twice cannot fire against it. +function confidentAgainst(description, query) { + const context = { id: "one", name: "Ctx", routingDescription: description }; + const index = buildIndex([{ id: "one", fields: routingFields(context, null, []) }]); + const [leader] = rank(index, query, { limit: Number.POSITIVE_INFINITY }); + return Boolean(leader) && isConfidentMatch({ ...leader, name: context.name }, query); +} + describe("assess", () => { it("calls a clear leader clear", () => { const decision = assess([candidate("winner", 10), candidate("other", 2)]); @@ -229,18 +250,33 @@ describe("isConfidentMatch", () => { it("does not let one concept spelled two ways count as two", () => { // `user_id` tokenizes to [user_id, user, id] and `user?` to [user], so both - // words agree on the single carried token `user`. Two parts of the request, - // one thing agreed on — which is one piece of evidence, not two. + // words agree on one concept. Two parts of the request, one thing agreed + // on — which is one piece of evidence, not two. + // + // Fixtured as `rank` actually fills `matched`, which is the whole reason + // this bites: a description containing `user_id` indexes every one of + // [user_id, user, id], so all three come back and the pairing had three + // separate things to spend two words on. Handing the leader `user` alone + // was the one shape in which the guard could never fire. assert.equal( - isConfidentMatch(candidate("Users", hit("user")), "what does user_id mean for a user?"), + isConfidentMatch( + candidate("Users", hit("user_id"), hit("user"), hit("id")), + "what does user_id mean for a user?" + ), false ); assert.equal( - isConfidentMatch(candidate("Docker", hit("docker")), "how do I run docker in docker-compose"), + isConfidentMatch( + candidate("Checkout API", hit("api"), hit("checkout-api"), hit("checkout")), + "is the api part of checkout-api?" + ), false ); assert.equal( - isConfidentMatch(candidate("Checkout API", hit("api")), "is the api part of checkout-api?"), + isConfidentMatch( + candidate("Docker", hit("docker"), hit("docker-compose"), hit("compose")), + "docker in docker-compose" + ), false ); // Two compounds sharing their only carried token is still one concept. @@ -255,6 +291,71 @@ describe("isConfidentMatch", () => { ); }); + it("still counts a second word that agreed on something of its own", () => { + // now sits. "How we run services under `docker-compose`" indexes + // [run, docker, docker-compose, compose]; the request "how do I run docker + // in docker-compose" names the compound twice — collapsed to one — but also + // agrees on `run`, which the description carries independently of it. Two + // parts of the request, two things agreed on, and the floor is met. + // + // That is the rule working, not escaping: `run` is a word in the request + // that a word in the description matched. Reading it as a false positive + // would mean the floor had to know which agreements are interesting, which + // is a stopword list with no principled place to stop. + const leader = candidate( + "Docker", + hit("run"), + hit("docker"), + hit("docker-compose"), + hit("compose") + ); + assert.equal(isConfidentMatch(leader, "how do I run docker in docker-compose"), true); + // Take that second agreement away and the compound is on its own again. + assert.equal(isConfidentMatch(leader, "docker in docker-compose"), false); + }); + + it("holds against `matched` as a real index fills it", () => { + // The same rule with no fixture in the way. Each row builds an index from a + // description, ranks the request against it, and hands `isConfidentMatch` + // whatever `rank` produced — which is the only version of this that proves + // anything about the bridge. + assert.equal( + confidentAgainst( + "How the `user_id` column is populated.", + "what does user_id mean for a user?" + ), + false + ); + assert.equal( + confidentAgainst( + "Everything about the `checkout-api` service.", + "is the api part of checkout-api?" + ), + false + ); + assert.equal( + confidentAgainst("How we run services under `docker-compose`.", "docker in docker-compose"), + false + ); + assert.equal(confidentAgainst("Notes about the user table.", "what does user mean"), false); + + // The controls: a request that agreed on two separate things still routes. + assert.equal( + confidentAgainst("checkout 5xx incidents and their causes.", "checkout 5xx"), + true + ); + // Including the case above once the request supplies a second agreement of + // its own — `run` is carried by the description independently of the + // compound, so this is two, and it is meant to be. + assert.equal( + confidentAgainst( + "How we run services under `docker-compose`.", + "how do I run docker in docker-compose" + ), + true + ); + }); + it("pairs the same way whichever order the words arrive in", () => { // `alpha-beta` can be spent on either token, so a first-come pairing counts // this as two one way round and one the other. Rewording a sentence must diff --git a/tests/routing.test.mjs b/tests/routing.test.mjs index 75cbcb8..b4ae063 100644 --- a/tests/routing.test.mjs +++ b/tests/routing.test.mjs @@ -216,6 +216,116 @@ describe("routing metadata", () => { assert.deepEqual(times, [...times].sort((left, right) => left - right)); }); + it("leaves a decision whose timestamp will not parse where it already was", async () => { + // `Date.parse` gives NaN for a hand-edited entry, a half-written one, or + // one whose writer spelled the field differently — and `NaN || 0` read that + // as 1970, which sorted it to the front. This runs on every write, so the + // move was permanent: one unreadable timestamp rewrote the chronology every + // "why did it route that way?" read is made from. + const record = await create("Undated", "undated check"); + const at = (minute) => new Date(Date.UTC(2026, 0, 1, 0, minute)).toISOString(); + await routing.noteDecision({ sessionId: "a", from: null, to: record.name, at: at(1) }); + await routing.noteDecision({ sessionId: "b", from: null, to: record.name, at: at(2) }); + await routing.noteDecision({ sessionId: "c", from: null, to: record.name, at: at(3) }); + + const file = path.join(home, "plugin-routing.json"); + const edited = JSON.parse(await readFile(file, "utf8")); + delete edited.decisions[1].at; + await writeFile(file, `${JSON.stringify(edited, null, 2)}\n`, "utf8"); + + // Any write re-caps the log, which is where the relocation happened. + await routing.noteDecision({ sessionId: "d", from: null, to: record.name, at: at(4) }); + const state = await routing.readRouting(); + assert.deepEqual( + state.decisions.map((decision) => decision.sessionId), + ["a", "b", "c", "d"] + ); + + // And it stays put however many writes follow, rather than drifting one + // place per write. + await routing.noteDecision({ sessionId: "e", from: null, to: record.name, at: at(5) }); + const again = await routing.readRouting(); + assert.deepEqual( + again.decisions.map((decision) => decision.sessionId), + ["a", "b", "c", "d", "e"] + ); + }); + + it("stops treating a refusal as a veto long before it stops discounting", () => { + // `declineFactor` decays over six weeks and hits exactly 1 only at the end + // of them. Reading "not exactly 1" as a bar on connecting unasked gave a + // refusal made a month and a half ago the same absolute force as one made + // this morning — at day 41 the multiplier is about 0.99. + const now = new Date("2026-03-01T00:00:00.000Z"); + const daysAgo = (days) => ({ + declines: { + one: { at: new Date(now.getTime() - days * 24 * 60 * 60 * 1000).toISOString(), count: 1 } + } + }); + + assert.equal(routing.hasLiveDecline(daysAgo(1), "one", now), true); + assert.equal(routing.hasLiveDecline(daysAgo(13), "one", now), true); + assert.equal(routing.hasLiveDecline(daysAgo(20), "one", now), false); + assert.equal(routing.hasLiveDecline(daysAgo(41), "one", now), false); + // Still a discount at that age, which is the whole point of separating them. + assert.ok(routing.declineFactor(daysAgo(20), "one", now) < 1); + + assert.equal(routing.hasLiveDecline({ declines: {} }, "one", now), false); + assert.equal(routing.hasLiveDecline({}, "one", now), false); + assert.equal(routing.hasLiveDecline({ declines: { one: { at: "nonsense" } } }, "one", now), false); + // A clock that jumped errs towards not acting, as `declineFactor` does. + assert.equal(routing.hasLiveDecline(daysAgo(-5), "one", now), true); + }); + + it("does not let the machine's own routes evict a window's settings", async () => { + // A session record holds what the user chose for that window — its mode + // override, and what they declined in it. Every writer used to need a + // person or a model; an automatic route needs neither and arrives at about + // one per new session, so creating a record for one turned the session cap + // into a shredder: twenty windows connecting themselves elsewhere evicted + // the record of a window that had been put in manual, `resolveMode` fell + // through to the default — `auto` — and a session where the user had turned + // routing off started routing itself again. + const record = await create("Evictions", "eviction check"); + await routing.setMode("manual", { id: "pinned" }); + await routing.noteDeclined(record.id, { id: "pinned" }); + + for (let index = 0; index < 40; index += 1) { + await routing.noteDecision({ + sessionId: `auto-${index}`, + from: null, + to: record.name, + automatic: true + }); + } + + const state = await routing.readRouting(); + assert.equal(routing.resolveMode(state, "pinned"), "manual"); + assert.deepEqual(state.sessions.pinned.declined, [record.id]); + assert.equal( + routing.switchPolicy(state, { id: "pinned", targetId: record.id, connectedId: null }).reason, + "manual-mode" + ); + // The routes themselves are still on the record; `decisions` is where a + // machine route belongs, and it is capped in a bucket of its own. + assert.equal( + state.decisions.filter((decision) => decision.automatic === true).length > 0, + true + ); + + // A window that already has a record still has its switches counted, so an + // automatic route in a session the user has touched is not invisible. + await routing.noteDecision({ + sessionId: "pinned", + from: null, + to: record.name, + automatic: true + }); + const after = await routing.readRouting(); + assert.equal(after.sessions.pinned.switches, 1); + assert.equal(routing.resolveMode(after, "pinned"), "manual"); + }); + it("enforces auto, ask, manual, declined, and already-connected policies", () => { const base = { mode: "ask", sessions: {} }; assert.equal(