From ee751b7dc8916f54dbfae8ab3738c1c0770094a5 Mon Sep 17 00:00:00 2001 From: Robbie Court Date: Wed, 15 Jul 2026 10:35:41 +0100 Subject: [PATCH] Make count-question auto-run authoritative and deterministic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The count-question auto-run was defeated by its own guard (it skipped whenever the planner had already planned any run_query) and, even when it fired, relied on the weak extractor to pull the count out of the result table — so "how many images of neurons with a part in the medulla" regressed to "you can run the query" (no number), and earlier had run PartsOf (28 subparts) reported as "28 images". - maybeInjectCountQueryStep no longer skips on a generic run_query. If the planner planned a run_query with the wrong query_type, it RETARGETS that step to the semantically-matched query (e.g. PartsOf -> ImagesNeurons); otherwise it injects one. Still skips only when a specialised connectivity / scRNA-seq / region-neuron-count macro already covers the count, and only on an unambiguous match. - The step is tagged count_query + count_noun + count_term; runStep reads the count straight from the run_query result and adds a deterministic evidence row ("VFB holds 226,524 images …"), so the number and its correct unit reach the synthesiser regardless of the extractor. Offline end-to-end test (mocked ELM/MCP) with the user's exact question incl. the deliberate "medualla" typo: runs ImagesNeurons (never PartsOf) and the evidence states "226,524 images". Suites: orchestrator 23, orchestrator_run 23; whole offline unit suite green; next lint clean. --- lib/orchestrator.mjs | 47 ++++++++++++++++++++++++++-- tests/unit/orchestrator.test.mjs | 8 +++-- tests/unit/orchestrator_run.test.mjs | 31 ++++++++++++++++++ 3 files changed, 81 insertions(+), 5 deletions(-) diff --git a/lib/orchestrator.mjs b/lib/orchestrator.mjs index 2033bde..042ebca 100644 --- a/lib/orchestrator.mjs +++ b/lib/orchestrator.mjs @@ -27,7 +27,7 @@ import { import { extractPublicationRefs } from './literatureRefs.mjs' import { isTermInfo, buildTermInfoDigest, termInfoToDigestText, unwrapTermInfo, parseReplacedBy, isDeprecatedRecord } from './termInfoDigest.mjs' import { synthGuidance } from './guidanceCards.mjs' -import { isIndividualImageQuery } from './queryTypes.mjs' +import { isIndividualImageQuery, querySemantics } from './queryTypes.mjs' const MAX_EXTRACT_CHARS = 6000 @@ -277,6 +277,26 @@ async function runStep(ledger, step, deps, paramsByName, models, log) { let parsed = parseMaybe(out) if (isInvestigationOutput(parsed)) parsed = buildInvestigationDirective(parsed) // don't dead-stop + // Deterministic count answer for auto-injected count steps: read the count + // straight from the run_query result so the number (and its correct unit) + // reaches the synthesiser regardless of what the weak extractor makes of the + // table. This is what makes "how many images …" report the actual number. + if (step.count_query && parsed && typeof parsed === 'object') { + const count = Number(parsed.count) + if (Number.isFinite(count) && count >= 0) { + const noun = step.count_noun || 'results' + const where = step.count_term ? ` with a part in / for ${step.count_term}` : '' + addEvidence(ledger, buildEvidenceRow({ + source: 'vfb', + claim: `VFB holds ${count.toLocaleString('en-US')} ${noun}${where}.`, + verbatim: JSON.stringify({ query_type: args.query_type, count, count_noun: noun }), + locator: { stepId: step.id, tool: step.tool } + })) + log({ run_step: step.id, tool: step.tool, answered: true, count, count_noun: noun }) + return + } + } + const ex = await extractAnswer({ question: ledger.question, answers: step.answers, result: parsed ?? out, tool: step.tool, deps, model: models.extract @@ -723,19 +743,40 @@ export function pickBestQueryForQuestion(question, digest) { export function maybeInjectCountQueryStep(ledger, question, log = () => {}) { const q = String(question || '') if (!COUNT_INTENT_RE.test(q)) return - if (ledger.plan.some(s => /run_query|region_neuron_count|connectivity|scrnaseq/i.test(s.tool))) return + // Skip only when a specialised macro already answers the count its own way. + // Do NOT skip on a generic run_query: the planner may have planned the WRONG + // query_type (e.g. PartsOf for an images question) — this injector is the + // authority on which query answers the count, so it corrects that instead. + if (ledger.plan.some(s => /region_neuron_count|connectivity|scrnaseq/i.test(s.tool))) return const terms = Object.values(ledger.terms || {}).filter(t => t.id && t.digest?.queries?.length) if (terms.length !== 1) return const t = terms[0] const best = pickBestQueryForQuestion(q, t.digest) if (!best?.query_type) return + const sem = querySemantics(best.query_type) + // Deterministic-count metadata: runStep reads the count straight from the + // result for these steps, so the number reaches the answer regardless of the + // weak extractor. + const countMeta = { count_query: true, count_noun: sem.countNoun, count_term: t.digest?.name || t.label || t.id } + // If the planner already planned a run_query, retarget it to the correct query + // (single, correct run_query) rather than adding a competing one. + const existing = ledger.plan.find(s => s.tool === 'vfb_run_query' && s.status !== 'satisfied') + if (existing) { + existing.args = { id: t.id, query_type: best.query_type } + existing.answers = [q] + Object.assign(existing, countMeta) + existing.note = `count question → run ${best.query_type} (retargeted)` + log({ retarget: 'vfb_run_query', query_type: best.query_type, id: t.id }) + return + } ledger.plan.push({ id: `cq${ledger.plan.length + 1}`, tool: 'vfb_run_query', answers: [q], args: { id: t.id, query_type: best.query_type }, status: 'pending', - note: `auto-injected count step (count question → run ${best.query_type})` + note: `auto-injected count step (count question → run ${best.query_type})`, + ...countMeta }) log({ inject: 'vfb_run_query', query_type: best.query_type, id: t.id }) } diff --git a/tests/unit/orchestrator.test.mjs b/tests/unit/orchestrator.test.mjs index 0ca628b..bc8f936 100644 --- a/tests/unit/orchestrator.test.mjs +++ b/tests/unit/orchestrator.test.mjs @@ -266,9 +266,13 @@ test('maybeInjectCountQueryStep: no-op for non-count questions and when a run_qu addTerm(l1, 'medulla', { id: 'FBbt_00003748', digest: MEDULLA_DIGEST }) maybeInjectCountQueryStep(l1, 'what is the medulla?') assert.equal(l1.plan.length, 0) - // already has a run_query step → don't duplicate - const l2 = setPlan(createLedger('q'), { intent: 'other', underspecified: false, clarifying_question: '', terms_to_resolve: ['medulla'], steps: [{ id: 's1', tool: 'vfb_run_query', answers: ['x'] }] }) + // a planner run_query with the WRONG query_type is RETARGETED (not duplicated) + // to the semantically-correct one — this is the "28 images" (PartsOf) fix. + const l2 = setPlan(createLedger('q'), { intent: 'other', underspecified: false, clarifying_question: '', terms_to_resolve: ['medulla'], steps: [{ id: 's1', tool: 'vfb_run_query', answers: ['x'], args: { id: 'FBbt_00003748', query_type: 'PartsOf' } }] }) addTerm(l2, 'medulla', { id: 'FBbt_00003748', digest: MEDULLA_DIGEST }) maybeInjectCountQueryStep(l2, MEDULLA_Q) assert.equal(l2.plan.length, 1) + assert.equal(l2.plan[0].tool, 'vfb_run_query') + assert.equal(l2.plan[0].args.query_type, 'ImagesNeurons') + assert.equal(l2.plan[0].count_query, true) }) diff --git a/tests/unit/orchestrator_run.test.mjs b/tests/unit/orchestrator_run.test.mjs index a30f5c0..3912a58 100644 --- a/tests/unit/orchestrator_run.test.mjs +++ b/tests/unit/orchestrator_run.test.mjs @@ -218,6 +218,37 @@ test('fast-path definitional question skips the planner call', async () => { assert.ok(deps.calls.tools.some(t => t.name === 'vfb_search_terms')) }) +test('count question: auto-runs the individual-image query and reports the count deterministically', async () => { + // The exact failing case (deliberate "medualla" typo). The digest offers both + // ImagesNeurons (images, count -1) and PartsOf (class, 28). The chat must run + // ImagesNeurons and state its count — never PartsOf, never "you can run it". + const termInfo = { + Name: 'medulla', Id: 'FBbt_00003748', + SuperTypes: ['Class', 'Anatomy', 'Synaptic_neuropil'], + Meta: { Name: '[medulla](FBbt_00003748)', Description: 'Optic-lobe neuropil.' }, + Queries: [ + { query: 'ImagesNeurons', label: 'Images of neurons with some part in medulla', count: -1, preview_results: { rows: [] } }, + { query: 'PartsOf', label: 'Parts of medulla', count: 28, preview_results: { rows: [] } } + ], + Publications: [] + } + const plan = { intent: 'other', underspecified: false, clarifying_question: '', terms_to_resolve: ['medulla'], steps: [] } + const deps = makeDeps({ + plan, + tools: { + vfb_search_terms: () => ({ response: { docs: [{ short_form: 'FBbt_00003748', label: 'medulla' }] } }), + vfb_get_term_info: () => termInfo, + vfb_run_query: (a) => ({ count: a.query_type === 'ImagesNeurons' ? 226524 : 28, headers: {}, rows: [] }) + } + }) + const r = await runHarness('how many images of neurons with a part in the medualla are available?', deps) + const ran = deps.calls.tools.filter(t => t.name === 'vfb_run_query').map(t => t.args.query_type) + assert.ok(ran.includes('ImagesNeurons'), `expected ImagesNeurons to run, ran: ${ran.join(',') || 'none'}`) + assert.ok(!ran.includes('PartsOf'), 'must never run the class query PartsOf for an images question') + const claims = r.ledger.evidence.map(e => e.claim).join(' | ') + assert.match(claims, /226,524 images/) +}) + test('pickBestTermId prefers exact synonym/label over the first fuzzy hit', () => { // "DAN" must bind to dopaminergic neuron (which carries the synonym), not the // first fuzzy result (mushroom body) — the bug seen live.