From aace891569e01537ac6cab8672a1e86a3f7d42f1 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 16:10:34 +0000 Subject: [PATCH 1/3] Sharpen the keyword index against the sentences it mis-read MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five everyday messages routed to a skill, and the tests now say so first: 'I work out every morning' reached the calculator, 'in summary, the trip was a success' the page reader, 'heute war ein schöner Tag' the clock, 'ich rechne damit' the calculator again, and 'my sister will find out sooner or later' the web search. Four were keywords an ordinary sentence contains by accident; the last two were triggers, so 'work out' now needs a number in the message and 'find out' no longer matches after an auxiliary. Keywords a skill's own triggers already match are gone too, and a test keeps them gone: unreachable, and they still dilute the idf of the terms that are not. Two more from the same read-through. A skill can decline carry-over with carry: false, which summarize-url does - carried onto a bare follow-up it offers the model a page reader and no page. And the retrieval index is built once per catalogue rather than once per message. Co-authored-by: Sebastian --- src/skills/arithmetic/SKILL.md | 12 +++---- src/skills/current-date/SKILL.md | 6 ++-- src/skills/load.test.ts | 30 ++++++++++++++++++ src/skills/load.ts | 10 +++++- src/skills/research-question/SKILL.md | 8 ++--- src/skills/retrieve.ts | 45 ++++++++++++++++++++------- src/skills/route.test.ts | 12 +++++++ src/skills/route.ts | 2 +- src/skills/summarize-url/SKILL.md | 9 +++--- src/skills/types.ts | 10 ++++++ 10 files changed, 113 insertions(+), 31 deletions(-) diff --git a/src/skills/arithmetic/SKILL.md b/src/skills/arithmetic/SKILL.md index 31ab193..8fe30e2 100644 --- a/src/skills/arithmetic/SKILL.md +++ b/src/skills/arithmetic/SKILL.md @@ -5,13 +5,12 @@ jarvis: priority: 30 tools: - calculator + # Only what the triggers miss, and nothing a sentence might contain by + # accident: bare `rechne` also starts "ich rechne damit, dass …". keywords: - - calculate - - work out - how much is - - square root - - percent of - - rechne + - rechne mir + - rechne aus - berechne - wie viel ist - quadratwurzel @@ -21,7 +20,8 @@ jarvis: - '\d+(\.\d+)?\s*(percent|per cent|%)\s*(of|off)' - '\b(times|multiplied by|divided by|plus|minus)\b.*\d' - '\b(square root|sqrt|to the power of|squared|cubed)\b' - - '\b(calculate|work out|compute)\b' + # A number has to be in the message, or this is "I work out every morning". + - '\b(calculate|work out|compute)\b.*\d' exemplars: - user: What is 6748 * 9? steps: diff --git a/src/skills/current-date/SKILL.md b/src/skills/current-date/SKILL.md index c9cc9f4..0916a85 100644 --- a/src/skills/current-date/SKILL.md +++ b/src/skills/current-date/SKILL.md @@ -5,16 +5,14 @@ jarvis: priority: 25 tools: - current_time + # Bare `heute` is in every second German sentence, so it is not here. keywords: - - date today - what day - - current year - - time right now - datum - welcher tag - welches jahr - uhrzeit - - heute + - wie spät triggers: - '\b(today|tonight|right now|at the moment|currently)\b' - "\\bwhat('s| is)? the (date|time|day)\\b" diff --git a/src/skills/load.test.ts b/src/skills/load.test.ts index 40527b4..73a3f0f 100644 --- a/src/skills/load.test.ts +++ b/src/skills/load.test.ts @@ -152,6 +152,21 @@ Body.` expect(parseSkill(source, 'example/SKILL.md').keywords).toEqual(['wetter', 'temperature outside']) }) + it('lets a skill refuse to be carried onto a follow-up', () => { + const source = `--- +name: example +description: A description. +jarvis: + carry: false +--- +Body.` + + expect(parseSkill(source, 'example/SKILL.md').carry).toBe(false) + // Carrying is the useful default: a follow-up usually wants the skill that + // answered the question before it. + expect(parseSkill(MINIMAL, 'example/SKILL.md').carry).toBe(true) + }) + it('rejects a keyword made only of stopwords', () => { const source = `--- name: example @@ -271,6 +286,21 @@ describe('the shipped skills', () => { }, ) + it.each(skills.map((skill) => [skill.name, skill] as const))( + '%s keeps no keyword its own triggers already match', + (_name, skill) => { + // Keywords are the second stage of routing and exist for what the first + // stage misses. One that a trigger already matches can never be reached, + // and it still dilutes the inverse document frequency of the terms that + // can — so it is not merely redundant, it makes the index worse. + for (const keyword of skill.keywords) { + expect( + skill.triggers.filter((trigger) => trigger.test(keyword)).map((trigger) => trigger.source), + ).toEqual([]) + } + }, + ) + it.each(skills.map((skill) => [skill.name, skill] as const))( '%s fits the context budget without being trimmed', (_name, skill) => { diff --git a/src/skills/load.ts b/src/skills/load.ts index 8990b1e..99237e2 100644 --- a/src/skills/load.ts +++ b/src/skills/load.ts @@ -129,6 +129,7 @@ interface SkillMetadata { keywords: string[] triggers: RegExp[] priority: number + carry: boolean jarvis: Record body: string } @@ -160,12 +161,18 @@ function parseMetadata(source: string, path: string): SkillMetadata { throw new Error(`${path}: "priority" must be a number`) } + const carry = jarvis.carry + if (carry !== undefined && typeof carry !== 'boolean') { + throw new Error(`${path}: "carry" must be a boolean`) + } + return { name: requireString(frontmatter.name, path, 'name'), description: requireString(frontmatter.description, path, 'description'), keywords: checkKeywords(stringArray(jarvis.keywords, path, 'keywords'), path), triggers: compileTriggers(stringArray(jarvis.triggers, path, 'triggers'), path), priority: priority ?? 0, + carry: carry ?? true, jarvis, body, } @@ -199,7 +206,7 @@ export function parseSkill(source: string, path: string): Skill { * prompt is kept in code, where it costs the model nothing. */ export function parseSkillEntry(source: string, path: string): SkillEntry { - const { name, description, keywords, triggers, priority } = parseMetadata(source, path) + const { name, description, keywords, triggers, priority, carry } = parseMetadata(source, path) let materialised: Skill | null = null return { @@ -208,6 +215,7 @@ export function parseSkillEntry(source: string, path: string): SkillEntry { keywords, triggers, priority, + carry, load: () => (materialised ??= parseSkill(source, path)), } } diff --git a/src/skills/research-question/SKILL.md b/src/skills/research-question/SKILL.md index 85929bb..c186f76 100644 --- a/src/skills/research-question/SKILL.md +++ b/src/skills/research-question/SKILL.md @@ -8,11 +8,9 @@ jarvis: - read_page keywords: - look it up - - find out - search the web - - who won - suche im netz - - schau nach + - schau im netz - finde heraus - wer hat gewonnen - aktuelle nachrichten @@ -20,7 +18,9 @@ jarvis: - '\b(latest|current|recent|news|today.s)\b' - '\bwho (is|was|are|won)\b' - '\b(20[2-9]\d)\b' - - '\b(look up|search for|find out|google)\b' + # `find out` only as a request. "My sister will find out" is not one. + - '\b(look up|search for|google)\b' + - '(? ({ +interface Index { + entries: Indexed[] + /** How many entries each term appears in, for the idf. */ + documentFrequency: Map +} + +/** + * Built once per catalogue rather than once per message. + * + * The catalogue is a module-level constant, so this is a cache with exactly one + * live key in practice; the WeakMap is what keeps the eval's throwaway + * catalogues from accumulating. + */ +const INDEXES = new WeakMap() + +function index(catalog: SkillEntry[]): Index { + const cached = INDEXES.get(catalog) + if (cached) return cached + + const entries: Indexed[] = catalog.map((entry) => ({ entry, phrases: entry.keywords .map((keyword) => ({ @@ -159,6 +177,16 @@ function index(catalog: SkillEntry[]): Indexed[] { .filter((phrase) => phrase.words.length > 0), fallback: entry.keywords.length === 0 ? [...new Set(contentTerms(entry.description))] : [], })) + + const documentFrequency = new Map() + for (const { phrases, fallback } of entries) { + const terms = new Set([...phrases.flatMap((phrase) => phrase.scored), ...fallback]) + for (const term of terms) documentFrequency.set(term, (documentFrequency.get(term) ?? 0) + 1) + } + + const built = { entries, documentFrequency } + INDEXES.set(catalog, built) + return built } /** @@ -181,14 +209,9 @@ export function search(message: string, catalog: SkillEntry[]): Retrieved[] { if (words.length === 0) return [] const query = new Set(contentTerms(message)) - const indexed = index(catalog) - const documentFrequency = new Map() - for (const { phrases, fallback } of indexed) { - const terms = new Set([...phrases.flatMap((phrase) => phrase.scored), ...fallback]) - for (const term of terms) documentFrequency.set(term, (documentFrequency.get(term) ?? 0) + 1) - } + const { entries, documentFrequency } = index(catalog) - const scored = indexed.map(({ entry, phrases, fallback }) => { + const scored = entries.map(({ entry, phrases, fallback }) => { const hits = phrases .filter((phrase) => phraseMatches(phrase.words, words)) .map((phrase) => ({ @@ -196,7 +219,7 @@ export function search(message: string, catalog: SkillEntry[]): Retrieved[] { // A longer phrase matching is a stronger signal than a shorter one, and // summing its terms says so without a separate length bonus. score: phrase.scored.reduce( - (total, word) => total + inverseFrequency(word, documentFrequency, indexed.length), + (total, word) => total + inverseFrequency(word, documentFrequency, entries.length), 0, ), })) @@ -208,7 +231,7 @@ export function search(message: string, catalog: SkillEntry[]): Retrieved[] { hits.push({ source: term, // Halved: the description was written to be read, not matched. - score: inverseFrequency(term, documentFrequency, indexed.length) / 2, + score: inverseFrequency(term, documentFrequency, entries.length) / 2, }) } } diff --git a/src/skills/route.test.ts b/src/skills/route.test.ts index 5317b46..e816e65 100644 --- a/src/skills/route.test.ts +++ b/src/skills/route.test.ts @@ -106,6 +106,12 @@ describe('routing nothing at all', () => { // Physics, not this afternoon: the word alone must not pull in the weather. 'What temperature does water boil at?', 'Erzähl mir einen Witz', + // Every one of these is a keyword doing something its author did not mean. + 'I work out every morning before breakfast', + 'In summary, the trip was a success', + 'Heute war ein wirklich schöner Tag', + 'Ich rechne damit, dass es morgen klappt', + 'My sister will find out sooner or later', ])('leaves %j to the model', (message) => { // Firing a tool-shaped skill on plain conversation is the failure mode that // makes a small model reach for tools it does not need. @@ -160,6 +166,12 @@ describe('keeping a skill across a follow-up', () => { it('forgets a skill that is no longer installed', () => { expect(routed('and in Lisbon?', { name: 'removed-skill', carried: 0 })).toBeNull() }) + + it('never carries a skill whose job needs something the follow-up lacks', () => { + // `summarize-url` declares carry: false. Carried onto a bare follow-up it + // would offer the model a page reader and no page. + expect(routed('and this one?', { name: 'summarize-url', carried: 0 })).toBeNull() + }) }) describe('isFollowUp', () => { diff --git a/src/skills/route.ts b/src/skills/route.ts index ff942d6..388ae2b 100644 --- a/src/skills/route.ts +++ b/src/skills/route.ts @@ -110,7 +110,7 @@ export function route(message: string, catalog: SkillEntry[], memory: SkillMemor } const resident = memory ? catalog.find((entry) => entry.name === memory.name) : undefined - if (resident && memory && memory.carried < MAX_CARRIED_TURNS && isFollowUp(message)) { + if (resident?.carry && memory && memory.carried < MAX_CARRIED_TURNS && isFollowUp(message)) { return { route: { entry: resident, reason: 'carried-over', matched: [] }, memory: { name: resident.name, carried: memory.carried + 1 }, diff --git a/src/skills/summarize-url/SKILL.md b/src/skills/summarize-url/SKILL.md index d380f21..63517cd 100644 --- a/src/skills/summarize-url/SKILL.md +++ b/src/skills/summarize-url/SKILL.md @@ -3,14 +3,15 @@ name: summarize-url description: Reads a page the user has linked to and summarises it. Use when the message contains a URL, or asks what a page says. jarvis: priority: 20 + # Never carried onto a follow-up: without a URL in the message this skill + # offers the model a page reader and no page. + carry: false tools: - read_page # Written as people write them: a keyword only matches contiguously, so - # "fasse zusammen" would miss "fasse mir die Seite zusammen". + # "fasse zusammen" would miss "fasse mir die Seite zusammen". Bare `summary` + # is not here either — it also begins "in summary, the trip was a success". keywords: - - summarise - - summarize - - summary - read this page - zusammenfassen - zusammenfassung diff --git a/src/skills/types.ts b/src/skills/types.ts index 839aaec..7189ffb 100644 --- a/src/skills/types.ts +++ b/src/skills/types.ts @@ -51,6 +51,15 @@ export interface Skill { exemplars: SkillExemplar[] /** Higher wins when several skills match. */ priority: number + /** + * Whether this skill may be carried onto a follow-up that matches nothing. + * + * Most should be: *and in Lisbon?* wants the skill that answered the question + * before it. Some cannot be, because their whole job depends on something the + * follow-up does not contain — `summarize-url` carried onto *and tomorrow?* + * offers the model a page reader and no page. + */ + carry: boolean /** Optional per-skill reasoning budget override. */ strategy?: StrategyId } @@ -69,6 +78,7 @@ export interface SkillEntry { keywords: string[] triggers: RegExp[] priority: number + carry: boolean /** Materialises the body and exemplars. Memoised, so calling it twice is free. */ load: () => Skill } From 7f78c3e30a2dea68f93cceaecb6c21d194ed9511 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 16:10:35 +0000 Subject: [PATCH 2/3] Stop the answer check reading a citation into what nobody fetched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three false positives found by re-reading it. A URL in an answer is only a citation when something was actually fetched - asked for a website, the URL is the answer - so both citation checks now stand down without a source. The evidence comes from the real conversation rather than the composed turns, so an exemplar's URL stays an invention, which is the citation this model is likeliest to get wrong. And a value that only renders in exponent form is not checked at all, where before an answer writing it out in full digits read as wrong. Also the typographic minus, which a model writing prose uses and Number does not: -5 was flagged for saying −5. Co-authored-by: Sebastian --- src/agent/loop.test.ts | 25 +++++++++++++++++++++ src/agent/loop.ts | 11 +++++++++- src/agent/review.test.ts | 33 ++++++++++++++++++++++++++-- src/agent/review.ts | 47 +++++++++++++++++++++++++++++----------- src/eval/runner.ts | 8 +++++-- src/store/chat.ts | 12 ++++++++-- 6 files changed, 116 insertions(+), 20 deletions(-) diff --git a/src/agent/loop.test.ts b/src/agent/loop.test.ts index 3998e64..3c681a9 100644 --- a/src/agent/loop.test.ts +++ b/src/agent/loop.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest' import type { LlmClient } from '@/llm/client' import { MAX_TOOL_ROUNDS } from '@/llm/config' import { defineTool } from '@/tools/types' +import { collectEvidence } from './review' import { runAgent } from './loop' import type { AgentCallbacks } from './loop' @@ -230,6 +231,30 @@ describe('checking the answer before returning it', () => { expect(hooks.onCorrection).toHaveBeenCalledWith(['missing-source']) }) + it('treats a URL copied out of a worked example as an invention', async () => { + // The turns a skill composes begin with its exemplars, and repeating their + // URLs instead of citing what was fetched is a failure a model this size + // makes. So the evidence comes from the real conversation, passed in. + const exemplar = 'Ama Osei.\n\nSource: https://exemplar.example/leadership' + const composed = [ + { role: 'user' as const, content: 'Who is the chief executive of Fictional Airways?' }, + { role: 'assistant' as const, content: exemplar }, + ...turns, + ] + const client = fakeClient([ + toolCall('web_search', 'query', 'Fictional Airways chief executive'), + 'copying the exampleAma Osei.\n\nSource: https://exemplar.example/leadership', + 'using what came backAma Osei.\n\nSource: https://fictionalairways.example/leadership', + ]) + + const result = await runAgent(client, composed, [search], callbacks(), { + evidence: collectEvidence(turns), + }) + + expect(result.review).toEqual({ found: ['invented-source'], corrected: true }) + expect(result.content).toContain('https://fictionalairways.example/leadership') + }) + it('can be switched off so the eval can measure what it is worth', async () => { const client = fakeClient([ toolCall('web_search', 'query', 'Fictional Airways chief executive'), diff --git a/src/agent/loop.ts b/src/agent/loop.ts index e797790..38d1279 100644 --- a/src/agent/loop.ts +++ b/src/agent/loop.ts @@ -9,6 +9,7 @@ import { correctionPrompt, reviewAnswer, type ReviewCheck, + type ReviewEvidence, type ReviewOutcome, } from './review' @@ -39,6 +40,14 @@ export interface AgentOptions { strategy?: GenerationStrategy /** Check the answer before returning it. On unless the eval turns it off. */ review?: boolean + /** + * What the answer may be checked against, gathered from the real conversation. + * + * Worth passing: `turns` starts with a skill's worked examples, and letting an + * exemplar's URLs count as evidence excuses the citation a small model is most + * likely to get wrong — the one it copied out of the example. + */ + evidence?: ReviewEvidence } /** @@ -104,7 +113,7 @@ export async function runAgent( const conversation = [...turns] const strategy = options.strategy ?? DEFAULT_STRATEGY const checking = options.review ?? true - const evidence = collectEvidence(turns) + const evidence = options.evidence ?? collectEvidence(turns) let last: AgentResult = { content: '', diff --git a/src/agent/review.test.ts b/src/agent/review.test.ts index b7c6654..ec7e374 100644 --- a/src/agent/review.test.ts +++ b/src/agent/review.test.ts @@ -51,6 +51,21 @@ describe('reviewAnswer', () => { expect(checks('About 0.333.', third)).toEqual([]) }) + it('accepts a negative written with the minus sign prose uses', () => { + const owed = evidence({ toolResults: [{ tool: 'calculator', result: '3 - 8 = -5' }] }) + + // U+2212, which a model writing prose reaches for and `Number` never does. + expect(checks('The result is \u22125.', owed)).toEqual([]) + }) + + it('checks nothing when the value only exists in exponent form', () => { + const huge = evidence({ toolResults: [{ tool: 'calculator', result: '10 ^ 21 = 1e+21' }] }) + + // `String` and `toFixed` both keep the `e`, so an answer that writes the + // number out in full would read as the wrong number. + expect(checks('That is 1,000,000,000,000,000,000,000.', huge)).toEqual([]) + }) + it('leaves a failed calculation alone', () => { const failed = evidence({ toolResults: [ @@ -132,6 +147,12 @@ describe('reviewAnswer', () => { expect(checks('It is 2026.', timed)).toEqual([]) }) + it('does not treat a URL as a citation when nothing was fetched', () => { + // *What is Anthropic's website* is answered with a URL, and that URL is + // the answer rather than a source for one. + expect(checks('It is https://anthropic.com.', evidence())).toEqual([]) + }) + it('accepts a URL the user supplied but no tool returned', () => { const failed = evidence({ knownUrls: ['https://example.com/pricing'], @@ -173,13 +194,21 @@ describe('collectEvidence', () => { { role: 'tool', content: 'https://exemplar.example/page' }, ]) - // The system turn and the exemplar tool turns are not evidence: a URL from a - // worked example is exactly the kind of thing the model should not cite. + // The system turn and any tool turn already in the history are not evidence. expect(collected).toEqual({ toolResults: [], knownUrls: ['https://example.com/pricing', 'https://example.com/old'], }) }) + + it('carries over the URLs an earlier reply cited', () => { + const history = [ + { role: 'user' as const, content: 'Who runs it?' }, + { role: 'assistant' as const, content: 'Ama Osei.\n\nSource: https://fictionalairways.example' }, + ] + + expect(collectEvidence(history).knownUrls).toEqual(['https://fictionalairways.example']) + }) }) describe('correctionPrompt', () => { diff --git a/src/agent/review.ts b/src/agent/review.ts index 1e8d4fe..1919637 100644 --- a/src/agent/review.ts +++ b/src/agent/review.ts @@ -48,11 +48,18 @@ function findUrls(text: string): string[] { return [...text.matchAll(URL_IN_TEXT)].map((match) => match[0].replace(TRAILING_PUNCTUATION, '')) } -/** Evidence as it stands before the first tool has run. */ -export function collectEvidence(turns: ChatTurn[]): ReviewEvidence { +/** + * Evidence as it stands before the first tool has run. + * + * Hand this the real conversation, not the turns the model is about to be sent: + * those begin with a skill's worked examples, and an exemplar's URLs are exactly + * what a model this size is most likely to repeat instead of citing what it + * fetched. A URL that only ever appeared in an example is an invention. + */ +export function collectEvidence(history: ChatTurn[]): ReviewEvidence { return { toolResults: [], - knownUrls: turns + knownUrls: history .filter((turn) => turn.role === 'user' || turn.role === 'assistant') .flatMap((turn) => findUrls(turn.content)), } @@ -104,6 +111,9 @@ function calculations(evidence: ReviewEvidence): { expression: string; value: nu /** Model output puts thousands separators in unpredictable places. */ const SEPARATORS = /[,\s_'’]/g +/** A model writing prose reaches for the typographic minus, `Number` does not. */ +const MINUS = /[\u2212\u2012\u2013\u2014]/g + /** * Every rendering of a number the answer may use. * @@ -114,7 +124,10 @@ const SEPARATORS = /[,\s_'’]/g */ function renderings(value: number): string[] { const exact = String(value) - if (exact.includes('e')) return [exact] + // Exponent form has no plain-decimal rendering to look for — `toFixed` keeps + // the `e` too — so a correct answer written out in full digits would read as + // a wrong one. Nothing to check rather than something to get wrong. + if (exact.includes('e')) return [] const all = new Set([exact]) if (!Number.isInteger(value)) { @@ -124,8 +137,11 @@ function renderings(value: number): string[] { } function statesNumber(answer: string, value: number): boolean { - const digits = answer.replace(SEPARATORS, '') - return renderings(value).some((rendering) => digits.includes(rendering)) + const renderable = renderings(value) + if (renderable.length === 0) return true + + const digits = answer.replace(MINUS, '-').replace(SEPARATORS, '') + return renderable.some((rendering) => digits.includes(rendering)) } /** A question back to the user, and a plain "I could not find it", cite nothing. */ @@ -161,24 +177,29 @@ export function reviewAnswer(answer: string, evidence: ReviewEvidence): ReviewFi }) } + // Both citation checks need something to have been fetched. Without that, a + // URL in an answer is not a citation of anything — it is the answer, as in + // *what is Anthropic's website* — and neither its absence nor its presence + // says the model got something wrong. const source = preferredSource(evidence) + if (!source) return findings + const known = [...evidence.knownUrls, ...evidence.toolResults.flatMap(({ result }) => findUrls(result))] .map(locate) .filter((entry): entry is Located => entry !== null) - const invented = findUrls(draft).find((url) => { - const cited = locate(url) - return cited !== null && !isGrounded(cited, known) + const cited = findUrls(draft) + const invented = cited.find((url) => { + const located = locate(url) + return located !== null && !isGrounded(located, known) }) if (invented) { findings.push({ check: 'invented-source', - instruction: source - ? `Nothing returned ${invented}. The source is ${source} — cite that one instead.` - : `Nothing returned ${invented}. Drop that link; no source was fetched.`, + instruction: `Nothing returned ${invented}. The source is ${source} — cite that one instead.`, }) - } else if (source && findUrls(draft).length === 0 && !NOTHING_TO_CITE.test(draft)) { + } else if (cited.length === 0 && !NOTHING_TO_CITE.test(draft)) { findings.push({ check: 'missing-source', instruction: `The answer cites no source. End it with "Source: ${source}".`, diff --git a/src/eval/runner.ts b/src/eval/runner.ts index 7a2cb30..75c2503 100644 --- a/src/eval/runner.ts +++ b/src/eval/runner.ts @@ -1,5 +1,5 @@ import { runAgent } from '@/agent/loop' -import type { ReviewCheck } from '@/agent/review' +import { collectEvidence, type ReviewCheck } from '@/agent/review' import type { LlmClient } from '@/llm/client' import type { GenerationStrategy } from '@/llm/config' import type { ChatTurn } from '@/llm/protocol' @@ -103,7 +103,11 @@ async function runAttempt( onToolEnd: () => {}, onRoundEnd: () => {}, }, - { strategy: activation?.strategy ?? arm.strategy, review: arm.review ?? true }, + { + strategy: activation?.strategy ?? arm.strategy, + review: arm.review ?? true, + evidence: collectEvidence(history(scenario)), + }, ) const names = calls.map((call) => call.name) diff --git a/src/store/chat.ts b/src/store/chat.ts index b2e9d65..50ee827 100644 --- a/src/store/chat.ts +++ b/src/store/chat.ts @@ -1,5 +1,6 @@ import { create } from 'zustand' import { runAgent } from '@/agent/loop' +import { collectEvidence } from '@/agent/review' import { LlmClient } from '@/llm/client' import type { ChatTurn, LoadProgress } from '@/llm/protocol' import { MODEL_ID } from '@/llm/config' @@ -172,10 +173,12 @@ export const useChatStore = create((set, get) => { patch((message) => ({ ...message, skill: { name: skill.name, reason, matched } })) } + const conversation = toHistory(history) + try { const result = await runAgent( getClient(), - composeTurns(toHistory(history), activation), + composeTurns(conversation, activation), activation?.tools ?? get().tools, { onPartial: ({ content, reasoning }) => patch((message) => ({ ...message, content, reasoning })), @@ -204,7 +207,12 @@ export const useChatStore = create((set, get) => { // record what was wrong with it before the tokens start replacing it. onCorrection: (found) => patch((message) => ({ ...message, review: { found, corrected: false } })), }, - activation?.strategy ? { strategy: activation.strategy } : {}, + { + // The real conversation, not the composed turns: a skill's exemplars + // carry URLs, and copying one out of an example is not a citation. + evidence: collectEvidence(conversation), + ...(activation?.strategy ? { strategy: activation.strategy } : {}), + }, ) patch((message) => ({ From 6338adb2f730643e1be57c475896abcc15caaa53 Mon Sep 17 00:00:00 2001 From: Cursor Agent Date: Mon, 24 Aug 2026 16:10:35 +0000 Subject: [PATCH 3/3] Write down the two keyword rules and what the citation checks need Co-authored-by: Sebastian --- .cursor/rules/model-skills.mdc | 8 ++++++-- .cursor/skills/debug-model-output/SKILL.md | 7 ++++++- .cursor/skills/write-model-skill/SKILL.md | 16 ++++++++++++---- README.md | 8 ++++++++ 4 files changed, 32 insertions(+), 7 deletions(-) diff --git a/.cursor/rules/model-skills.mdc b/.cursor/rules/model-skills.mdc index da1cacd..fff2f82 100644 --- a/.cursor/rules/model-skills.mdc +++ b/.cursor/rules/model-skills.mdc @@ -15,8 +15,12 @@ reading now. Read `.cursor/skills/write-model-skill/SKILL.md` before changing on - Routing is three stages — triggers, then a keyword search, then carry-over onto a follow-up — and all three run in code. The catalogue never enters the prompt, so an installed skill costs the model nothing until it routes. -- `keywords` match as contiguous phrases over the words as written. Prefer a phrase to a word, and - write it the way a user would type it. +- `keywords` match as contiguous phrases over the words as written. Prefer a phrase to a word, write + it the way a user would type it, and never use one an ordinary sentence might contain: `work out` + once routed "I work out every morning" to the calculator. A keyword its own triggers already match + is rejected by a test. +- `carry: false` when a skill needs something a follow-up fragment cannot contain, as `summarize-url` + needs a URL. - Only the skill that routed is materialised. Do not reach for `SkillEntry.load()` anywhere routing can see it; that is what makes the library free to grow. - A new or changed skill needs routing cases in `route.test.ts` and a scenario in diff --git a/.cursor/skills/debug-model-output/SKILL.md b/.cursor/skills/debug-model-output/SKILL.md index 9d4515d..c30d78e 100644 --- a/.cursor/skills/debug-model-output/SKILL.md +++ b/.cursor/skills/debug-model-output/SKILL.md @@ -59,7 +59,12 @@ Three things follow from that, and all three are easy to undo by accident: and trains the user to ignore the label. `review.test.ts` pins the shy cases — a clarifying question, a rounded decimal, a source carried over from an earlier turn — and they are the point. - **Only successful tool results become evidence.** A failed fetch has nothing to check against, and - demanding a citation for a page that never loaded is worse than saying nothing. + demanding a citation for a page that never loaded is worse than saying nothing. For the same + reason both citation checks stand down when nothing was fetched at all: asked for a website, the + URL is the answer rather than a source for one. +- **The evidence comes from the real conversation, passed in as `options.evidence`.** The turns the + model is sent start with a skill's exemplars, and letting their URLs count as evidence would excuse + the citation this model is likeliest to get wrong — the one it copied out of the example. `pnpm test` covers all of it: the checks are pure functions and `loop.test.ts` drives the correction round with a scripted client, so none of this needs a GPU. diff --git a/.cursor/skills/write-model-skill/SKILL.md b/.cursor/skills/write-model-skill/SKILL.md index 5c3c569..442ab29 100644 --- a/.cursor/skills/write-model-skill/SKILL.md +++ b/.cursor/skills/write-model-skill/SKILL.md @@ -61,10 +61,18 @@ Call `calculator` for the arithmetic. Do not work it out yourself. spends exactly the capacity the skill exists to conserve. - **`keywords` are the second stage of routing**, searched by `retrieve.ts` when no trigger fires. They match as phrases, over the words as written, so write them the way people write them: - `fasse zusammen` never matches "fasse mir die Seite zusammen". Prefer a phrase to a word — - `temperature outside`, not `temperature`, which also means the one water boils at. This is where a - skill gets reach into German, since every trigger in the library is English. A keyword made only - of stopwords is rejected at load. + `fasse zusammen` never matches "fasse mir die Seite zusammen". A keyword made only of stopwords is + rejected at load, and a test rejects any keyword the skill's own triggers already match — it could + never be reached and it still dilutes the idf of the ones that can. Two rules beyond that: + - **Prefer a phrase to a word.** `temperature outside`, not `temperature`, which also means the one + water boils at. + - **Never a word a sentence contains by accident.** `work out` routed "I work out every morning" to + the calculator; `summary` routed "in summary, the trip was a success" to the page reader; bare + `heute` routed "heute war ein schöner Tag" to the clock. Add the counter-example to the + routes-to-nothing corpus in `route.test.ts` when you are unsure. +- **`carry: false`** stops a skill being carried onto a follow-up. Set it when the skill's job needs + something a fragment cannot contain — `summarize-url` needs a URL, and carried onto "and tomorrow?" + it offers the model a page reader and no page. - **`tools` narrows what the model sees**, because accuracy falls as the visible tool list grows. An empty or absent list means no restriction. Names must match real tools or they are dropped silently. diff --git a/README.md b/README.md index 481f7a8..de40936 100644 --- a/README.md +++ b/README.md @@ -287,6 +287,11 @@ Retrieval is lexical rather than semantic on purpose. RAG-MCP shows semantic ret **What is searched is curated, and that is not a detail.** Retrieving over the `description` is the obvious move and a trap: `temperature` appears in the weather description, so a bag-of-words match fires the weather skill on _what temperature does water boil at_. Keywords are written to be matched instead, as phrases, over the words as written — dropping stopwords first would quietly turn `how warm` into `warm` and fire the weather skill on a bowl of soup. A skill that declares no keywords falls back to its description and needs two terms to match, because prose nobody wrote for a router is weaker evidence. +Two rules keep the index sharp, and both are enforced rather than advised: + +- **A keyword may not be something a trigger already matches.** It could never be reached, and it would still dilute the inverse document frequency of the terms that can — so a redundant keyword does not merely sit there, it makes the index worse. A test walks every shipped skill's keywords past its own triggers. +- **A keyword may not be a word a sentence contains by accident.** This is not hypothetical: `work out` routed _I work out every morning_ to the calculator, `summary` routed _in summary, the trip was a success_ to the page reader, bare `heute` routed _heute war ein schöner Tag_ to the clock, and `rechne` routed _ich rechne damit, dass es klappt_ to the calculator. All four are now in the corpus of messages that must route to nothing. + ### Removing what is not needed A skill that keeps applying to turns it has nothing to do with is worse than no skill: it spends context and narrows the tool list on a request that needed neither. So carry-over is deliberately hard to enter and easy to leave. @@ -294,6 +299,7 @@ A skill that keeps applying to turns it has nothing to do with is worse than no - A continuation has to either **say so** (`and`, `und`, `what about`) or be **too short to be asking anything of its own**. Length alone is not enough, and this is where the mechanism would turn harmful: _what is the capital of France?_ is six words, and answering it with the weather skill's exemplars resident would send the model searching for a fact it already knows. - It survives **two turns** on carry-over alone. Past that it has stopped being a continuation and become a default. - It is dropped the moment another skill matches, the message asks something fresh, the turn closes the exchange (_thanks_), or a new chat starts. +- **A skill can refuse to be carried at all** with `carry: false`. `summarize-url` does, because its job is a URL the follow-up does not contain: carried onto _and tomorrow?_ it would hand the model a page reader and no page. The resident skill is read back off the transcript rather than kept in a counter of its own, so rerunning a reply rewinds it too — a counter held to one side would still be carrying the turn it just discarded. @@ -317,6 +323,8 @@ A failed check costs one further generation. The model is handed its own draft a They are also deliberately shy. A clarifying question is asked for no citation; a long decimal quoted to fewer places counts as the calculator's number; citing the site when a page on it was read is close enough; a URL from an earlier reply is not an invention. Every check would rather miss a mistake than invent one, because a check that fires on a correct answer costs a generation and teaches you to ignore the whole mechanism. +Both citation checks need something to have actually been fetched. Without that, a URL in an answer is not a citation at all — asked _what is Anthropic's website_, the URL **is** the answer — so neither its presence nor its absence says anything went wrong. What the checks compare against is the real conversation rather than the turns the model was sent: those begin with a skill's worked examples, and repeating an exemplar's URL instead of the one that came back is precisely the mistake a 0.8B model makes. + The interface says what happened rather than quietly rewriting the reply. While the corrected answer streams in it is labelled with what is being fixed, and afterwards it carries `corrected` — claimed only for an answer that now passes every check — or `flagged`, naming what is still wrong with the text on screen. An answer half fixed and advertised as corrected would be worse than no check at all. ## Measuring changes