From 3967e0f3eb80b8ed41c761bc53cb1fcbe13e3017 Mon Sep 17 00:00:00 2001 From: Yusuf GUNDOGDU Date: Mon, 7 Sep 2026 20:10:55 +0300 Subject: [PATCH] fix(agent): a stop that read nothing is answered for whoever stopped MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The drive holds a sentence written for exactly one ending — "Read it yourself. Call inspect_schema for the tables and their columns, and inspect_plan for how a statement will run" — and sent it only to a model whose profile asked for it. One profile of twenty-eight does. A model nobody has measured has no profile at all, so the model most in need of the sentence was the one guaranteed not to receive it. Measured across the sweep behind 0.14.1: three hundred runs ended `model-stopped` with `no-report`, and half of them — a hundred and fifty — had called no tool at all. Ninety-four of those hundred and fifty were ended without the drive saying anything: `mistral-small3.1:24b` 51, `granite3.3:8b` 41, `mistral:7b` 40, `gpt-oss:20b` 18, none of them on the shipped roster. A hundred and ten of the hundred and fifty were on `investigation`, the first surface a model meets. The gate's own comment had already argued the cost away. It fires only where `anyToolCalled` is false, and `compose_report` is one of the tools that counts, so a run reaching it composed no report and has already earned `no-report` — "the turn cannot cost a pass". A bound that cannot protect a passing run is not protecting anything, and what this one withheld was a sentence on a run already lost. So the gate now reads `answersUnreadStop` rather than `retriesUnreadStop`. The two differ on one thing: a profile stating `false` is a measurement and is obeyed, while no profile at all is the absence of a measurement and is answered with the turn. This is the same split `planStatementAsksFor` made beside `planStatementRetriesFor`, and the fourth setting found in this shape. All twenty-eight shipped entries state the field, so no shipped model's turn count moves. Nineteen existing tests turned red on it, every one of them a run that stops having called nothing — which is the change reaching exactly what it was aimed at. Each gets the extra scripted turn the drive now spends; `tests/evals/real-model.ts` holds the shared warm-up that accounted for several. The pair that pins the behaviour lives beside the tests for the gate it belongs to, and both halves were confirmed to bite by mutation: reading an absent profile as `false` again fails one, ignoring a stated `false` fails the other. `docs/llms/model-tuning.md` gains the section this makes necessary — the last column of the settings table is the compiled default, and two settings answer differently for a model with no entry — and `docs/AGENT.md`, `docs/BACKLOG.md` B65 and the notes on `profile.ts` and `models/index.ts` are corrected where they said the absent case resolves to the defaults. --- docs/AGENT.md | 2 +- docs/BACKLOG.md | 4 +- docs/llms/model-tuning.md | 19 ++++- src/lib/agent/investigation.ts | 10 +-- src/lib/agent/models/index.ts | 29 +++++++- src/lib/agent/models/profile.ts | 20 ++++-- tests/evals/database-assessment.test.ts | 1 + tests/evals/injection.test.ts | 13 +++- tests/evals/investigation-arc.test.ts | 12 ++-- tests/evals/legacy-surface-coverage.test.ts | 9 ++- tests/evals/query-optimization.test.ts | 3 + tests/evals/real-model.ts | 4 +- tests/evals/report-citation.test.ts | 1 + tests/isolated/agent-investigation.test.ts | 77 +++++++++++++++++++-- tests/unit/lib/agent/model-profiles.test.ts | 19 +++++ 15 files changed, 190 insertions(+), 33 deletions(-) diff --git a/docs/AGENT.md b/docs/AGENT.md index 566333ab..ea1d803e 100644 --- a/docs/AGENT.md +++ b/docs/AGENT.md @@ -143,7 +143,7 @@ run by asking `GET /api/agent/config`, the same way it discovers the storage mod | `LIBREDB_AGENT_THREAD_CONTEXT` | unset (on) | Whether a run may be told about the **conversation** it belongs to. A follow-up asked on the same connection continues the previous run's thread: the earlier steps' objectives and the most recent step's report are derived server-side from those runs' own ledgers and handed to the model fenced. Set `false`/`off`/`0` where no question's context may reach another. Every run then opens on its own and the rail SAYS so — a user who asks a follow-up is told the conversation is switched off on this server, rather than being left to infer it from an answer that does not resolve. `GET /api/agent/config` reports the state to an **admin** session, beside `modelTuning` and for the same reason: an operator who switches something off must not hear silence, and `curl` is how they check. It is deliberately not sent to every session — the sentence a USER needs comes from the run's own `thread.declined` at the moment their follow-up was not read as one, which is where it means something. An unrecognized value warns and is ignored, the same two-sided rule `LIBREDB_AGENT_ENABLED` follows: a typo must neither take a working surface away nor turn one on. It is the operator's counterpart to the control the user already has — the rail names the run being continued and offers "new conversation" beside it. | | `WORKFLOW_TARGET_WORLD` | unset (`local`) | Durable backend for run state. Exactly two values are accepted: `local` (zero-config, on-disk, **single instance**) and `@workflow/world-postgres` (opt-in, multi-replica, needs `WORKFLOW_POSTGRES_URL`). Anything else is **refused**, not defaulted. | | `AGENT_MODEL_TURN_TIMEOUT_MS` | unset (`90000`) | How long **one** model call may take before the drive stops waiting for it. Raise it for a LOCAL model: the default was chosen against hosted APIs, where a turn lands in seconds and a 90-second wait only ever means a request that is not coming back. Measured across 25 Ollama models on six surfaces, **nine** runs ended `model-timeout` with the model still working — one of them a reasoning model in plan mode, which holds no tools at all, cut 92 s into its **first** turn with a zero-event ledger. Those runs are scored as having answered nothing, which is a fact about this ceiling and not about the model. A value that is not a positive whole number is **ignored** and the default stands; a value is capped just under half the smallest workflow deadline, because a run has to be able to take two turns to finish. | -| `AGENT_MODEL_TUNING_PATH` | unset | A JSON document of measured per-model settings, layered over the ones Studio ships with. Studio carries a document recording what specific models were measured under — turn limit, how many readings before it is asked to report, whether an empty turn is asked again — and a model not named in it is driven with the defaults, which is the honest treatment of a model nobody has measured. This is how a model Studio has never measured gets settings somebody else measured: mount a file in the same shape and restart, with no Studio release and no code change. Merged **per model and whole** — an entry here replaces the shipped entry for that model rather than contributing one field to it, because half of one measurement beside half of another is a configuration nobody has run. A file that is missing, unreadable or off-schema is **ignored** and the shipped measurements stand — which is the one setting here that fails **open**, so it is also the one that reports itself: `GET /api/agent/config` tells an **admin** session what became of the document (`{"modelTuning":{"state":"applied"\|"ignored"\|"unset",…}}`, with the path and the parser's reason), because an operator who mounts a file and is told nothing will believe it is in force. It carries numbers and switches only: the sentences the drive says to a model stay in Studio, so supplying this file cannot change what Studio tells a model. On Kubernetes the chart mounts it for you — see `agent.modelTuning.*` in [`charts/libredb-studio/README.md`](../charts/libredb-studio/README.md). The document's own contract — every setting, its bounds, what happens to a key this build does not implement, and the example to start from — is [`docs/llms/model-tuning.md`](llms/model-tuning.md). | +| `AGENT_MODEL_TUNING_PATH` | unset | A JSON document of measured per-model settings, layered over the ones Studio ships with. Studio carries a document recording what specific models were measured under — turn limit, how many readings before it is asked to report, whether an empty turn is asked again — and a model not named in it is driven with the defaults, which is the honest treatment of a model nobody has measured — bar two settings whose gates are reachable only on a run that has already fallen short, where an absent entry is read as the absence it is rather than as a value somebody wrote. This is how a model Studio has never measured gets settings somebody else measured: mount a file in the same shape and restart, with no Studio release and no code change. Merged **per model and whole** — an entry here replaces the shipped entry for that model rather than contributing one field to it, because half of one measurement beside half of another is a configuration nobody has run. A file that is missing, unreadable or off-schema is **ignored** and the shipped measurements stand — which is the one setting here that fails **open**, so it is also the one that reports itself: `GET /api/agent/config` tells an **admin** session what became of the document (`{"modelTuning":{"state":"applied"\|"ignored"\|"unset",…}}`, with the path and the parser's reason), because an operator who mounts a file and is told nothing will believe it is in force. It carries numbers and switches only: the sentences the drive says to a model stay in Studio, so supplying this file cannot change what Studio tells a model. On Kubernetes the chart mounts it for you — see `agent.modelTuning.*` in [`charts/libredb-studio/README.md`](../charts/libredb-studio/README.md). The document's own contract — every setting, its bounds, what happens to a key this build does not implement, and the example to start from — is [`docs/llms/model-tuning.md`](llms/model-tuning.md). | | `WORKFLOW_LOCAL_DATA_DIR` | unset — but the packaged artifacts set it: `/app/data/workflow` from the Helm chart and (from an app version later than `0.11.0`) the container image, `~/.libredb-studio/workflow-data` under `npx`. The SDK's own fallback, which those replace, is `.workflow-data` relative to the working directory. | Where the `local` backend keeps run state, and therefore the second condition above. See [Deployment](#deployment) — the SDK's fallback is wrong in a container and wrong under `npx`, so no artifact leaves it in force. | The refusal is not pedantry. The workflow runtime reads that variable itself and treats any value diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index da8a8a07..b860ec32 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -1703,7 +1703,9 @@ switches therefore spends two extra turns rather than one, and a model carrying its `retryEmptyTurn: false` overridden by a switch that argues for something else. Live on `nemotron3:33b`, whose entry records `retryEmptyTurn: false` and whose empty turns are -asked again anyway. Pinned as it behaves in `tests/isolated/agent-investigation.test.ts` rather +asked again anyway — and, since the gate began reading `answersUnreadStop` rather than +`retriesUnreadStop`, on every model with no entry at all, which is the same subsumption over a +wider set. Pinned as it behaves in `tests/isolated/agent-investigation.test.ts` rather than repaired, because the repair — narrowing the gate to a turn with text in it — changes the behaviour the five passing query-optimization runs were measured under, and this repository does not move a measured cell without re-measuring it. diff --git a/docs/llms/model-tuning.md b/docs/llms/model-tuning.md index abde70ae..1d97135a 100644 --- a/docs/llms/model-tuning.md +++ b/docs/llms/model-tuning.md @@ -3,7 +3,9 @@ Studio ships a document recording what specific models were measured under — how long one of their turns may take, how many readings they may take before being asked to report, whether an empty turn is worth asking again. A model it does not name is driven with the compiled defaults, -which is the honest treatment of a model nobody has measured. +which is the honest treatment of a model nobody has measured — bar the two settings noted under +[the settings](#the-settings), where an absent entry is read as the absence it is rather than as a +`false` somebody wrote. This page is how you supply your own, with no new Studio release and no code change. It is the other half of [`testing-your-own.md`](testing-your-own.md): that page is how you measure a model, @@ -113,11 +115,11 @@ Every one is optional. What you do not state resolves to the compiled default in | `perWorkflow` | the same object, per workflow id | sampling for named surfaces only — the narrowest an override gets | — | | `unreportedCallCeiling` | integer 1–100 | how many calls it may make without reporting before the run is narrowed to the tools that would finish it | `12` | | `reportReminderLimit` | integer 0–5 | how many times a turn with no call and no report may be answered with the report reminder | `1` | -| `planStatementRetries` | integer 0–5 | extra turns a PLAN run gets when its prose named neither a statement nor a refusal | `0` | +| `planStatementRetries` | integer 0–5 | extra turns a PLAN run gets when its prose named neither a statement nor a refusal | `0` — but see below | | `presentReminderLimit` | integer 0–5 | how many times a report may be held to ask for the answer that belongs beside it | `1` | | `verdictHoldLimit` | integer 0–5 | how many times a report whose own verdict would REJECT it may be held and told why — the third of the reminder bounds. A run about to pass never reaches this hold, so raising it costs turns only on a run that has already lost | `2` | | `retryEmptyTurn` | boolean | whether a turn that came back EMPTY is asked once more before the run is ended | `false` | -| `retryUnreadStop` | boolean | whether a run that stopped having CALLED NOTHING is told once to read the database itself, instead of being ended — it subsumes `retryEmptyTurn`, since the gate asks what was called and not what was said | `false` | +| `retryUnreadStop` | boolean | whether a run that stopped having CALLED NOTHING is told once to read the database itself, instead of being ended — it subsumes `retryEmptyTurn`, since the gate asks what was called and not what was said | `false` — but see below | | `suppressPlanReasoning` | boolean | whether this model's PLAN turn asks the endpoint for no reasoning at all — reaches the OpenAI-compatible adapter only (`openai`, `ollama`, `custom`), so it is a no-op on `gemini` | `false` | | `suppressAgentReasoning` | boolean | the same, for this model's AGENT turns — for a model that either answers at once or thinks until the wall, which `turnTimeoutMs` does not address because a turn spent thinking finds the new wall too. Same adapters, same no-op on `gemini` | `false` | | `refusalExamples` | boolean | whether a refused call is handed a worked example built from this run's ledger | `false` | @@ -136,6 +138,17 @@ your model starts losing the schema block or ending early; raise it if your mode conversations are long enough to be truncating. Either way the answer comes from driving it, which is what [`testing-your-own.md`](testing-your-own.md) is for. +**Two settings answer differently for a model with NO entry at all**, and the last column above is +the compiled default rather than what an unnamed model gets. `planStatementRetries` and +`retryUnreadStop` each buy a single extra turn, and each gate is reachable only on a run that has +already fallen short — one where the closing prose named no statement, one where the model stopped +having called nothing. Neither can turn a pass into a failure, so withholding them from a model +nobody has measured only guarantees that the model most in need of the sentence is the one certain +not to receive it. A model with no entry is therefore offered both: one plan ask +(`planStatementAsksFor`) and one answer to a stop that read nothing (`answersUnreadStop`), both in +[`src/lib/agent/models/index.ts`](../../src/lib/agent/models/index.ts). **A stated value still +wins** — a `0` or a `false` you write here is a measurement, and Studio does not overrule it. + ## The rules **Merged per model and WHOLE.** An entry replaces the shipped entry for that model rather than diff --git a/src/lib/agent/investigation.ts b/src/lib/agent/investigation.ts index 73bb99d3..e2269d9e 100644 --- a/src/lib/agent/investigation.ts +++ b/src/lib/agent/investigation.ts @@ -66,7 +66,7 @@ import { presentReminderLimitFor, verdictHoldLimitFor, retriesEmptyTurn, - retriesUnreadStop, + answersUnreadStop, suppressesAgentReasoning, suppressesPlanReasoning, turnTimeoutMsFor, @@ -3516,8 +3516,10 @@ export async function runInvestigation( Granted only where the run has lost anyway. `compose_report` is one of the tools `anyToolCalled` counts, so a run reaching here with it false composed no report and - has already earned `no-report`; the turn cannot cost a pass. Once, and only for a - model whose ledger asked twice. + has already earned `no-report`; the turn cannot cost a pass. Once, and — because a + bound that cannot protect a passing run protects nothing — offered to whoever needs + it: `answersUnreadStop` reads a stated `false` as the measurement it is and an absent + profile as the absence it is, so a model nobody has measured is told to read. */ if ( record.mode === "agent" && @@ -3535,7 +3537,7 @@ export async function runInvestigation( // and spends the very turn this retry bought: the #350/#356 defect, paid for once. holdsTool("inspect_schema") && holdsTool("inspect_plan") && - retriesUnreadStop(model.modelId) + answersUnreadStop(model.modelId) ) { unreadStopRetried = true; messages.push(...turn.assistantMessages); diff --git a/src/lib/agent/models/index.ts b/src/lib/agent/models/index.ts index ae8e4046..87162cce 100644 --- a/src/lib/agent/models/index.ts +++ b/src/lib/agent/models/index.ts @@ -28,7 +28,10 @@ * in the document; that is a better trade than carrying it empty until then. * * A model nobody has measured therefore resolves exactly to the compiled defaults, which is the - * honest treatment of a model nobody has measured. + * honest treatment of a model nobody has measured. Two resolvers below — `planStatementAsksFor` + * and `answersUnreadStop` — deliberately do not, and each says why in its own note: they feed + * gates reachable only on a run that has already fallen short, where a default cannot protect a + * passing run and so protects nothing. A stated value still wins in both. */ import { AGENT_THREAD_CONTEXT_MAX_CHARS } from "../execution-policy"; @@ -157,6 +160,30 @@ export function retriesUnreadStop(modelId: string): boolean { return resolve(modelId, "retryUnreadStop") ?? DEFAULT_RETRY_UNREAD_STOP; } +/** + * The same question for a model NOBODY HAS MEASURED: the sentence is sent rather than withheld. + * + * `retriesUnreadStop` folds two different answers into one, exactly as `planStatementRetriesFor` + * did before `planStatementAsksFor` split them: a profile stating `false` is a measurement, and no + * profile at all is the absence of one. Reading the absence as a `false` withheld the drive's own + * sentence — "Read it yourself. Call inspect_schema for the tables and their columns, and + * inspect_plan for how a statement will run" — from precisely the models nobody had measured yet, + * which is to say from every model this project has not reached. + * + * The gate it feeds fires only where no tool was called, so the run composed no report and has + * already earned `no-report`; it provably cannot cost a pass. Measured across the sweep behind + * 0.14.1: three hundred runs ended `model-stopped` with `no-report`, a hundred and fifty of them + * having called no tool at all, and ninety-four of those hundred and fifty were ended without the + * drive saying anything — `mistral-small3.1:24b` 51, `granite3.3:8b` 41, `mistral:7b` 40, + * `gpt-oss:20b` 18, a hundred and ten of them on `investigation`, the first surface a model meets. + * + * A measured model is untouched: all twenty-eight profiles state the field, so no shipped model's + * turn count moves. + */ +export function answersUnreadStop(modelId: string): boolean { + return resolve(modelId, "retryUnreadStop") ?? true; +} + /** * Whether this model's plan turn is told to spend nothing on reasoning. * diff --git a/src/lib/agent/models/profile.ts b/src/lib/agent/models/profile.ts index 15667eba..0df40d5a 100644 --- a/src/lib/agent/models/profile.ts +++ b/src/lib/agent/models/profile.ts @@ -99,8 +99,12 @@ export interface AgentModelProfile { * run that called nothing composed no report and has already earned `no-report`; the turn is * spent on a run that has lost. It cannot cost a pass, only recover a failure. * - * Off by default even so. The ten models locked at 300/300 were measured without it, and a - * drive-wide change is twice how this repository has handed back cells it had won. + * A stated `false` is obeyed even so. The ten models locked at 300/300 were measured without + * it, and overruling a measurement is twice how this repository has handed back cells it had + * won. What is NOT obeyed is an absent entry, which is not a measurement: the drive asks + * `answersUnreadStop`, not `retriesUnreadStop`, and that resolver offers the turn to a model + * nobody has measured. All twenty-eight shipped entries state this field, so no shipped model's + * turn count moved when the two were split. * * It SUBSUMES `retryEmptyTurn`, and that is a property of the gate rather than of the name. * The condition is "called nothing", with no test on what was said, so an empty completion @@ -299,10 +303,14 @@ export const DEFAULT_RETRY_EMPTY_TURN = false; /** * A run that stops having called nothing keeps its ending, unless a model's ledger asked. * - * Off despite being free to grant — the turn is spent on a run whose verdict is already - * `no-report` — because "free" is an argument about cost, not about wording. The sentence sent - * is read by the model and acted on by it, so it is a measured value like every other, and it - * belongs to the models measured with it rather than to all of them at once. + * Off, and it stays off for every model that states it: the sentence sent is read by the model + * and acted on by it, so it is a measured value like every other and belongs to the models + * measured with it rather than to all of them at once. + * + * This constant is what a STATED field falls back to, and every shipped entry states the field, + * so nothing resolves through it today. It is not what a model with no entry gets — that is + * `answersUnreadStop`, which offers the turn, because on a run already earning `no-report` a + * default cannot protect a passing run and so protects nothing. */ export const DEFAULT_RETRY_UNREAD_STOP = false; diff --git a/tests/evals/database-assessment.test.ts b/tests/evals/database-assessment.test.ts index b2b0b678..a4de7171 100644 --- a/tests/evals/database-assessment.test.ts +++ b/tests/evals/database-assessment.test.ts @@ -153,6 +153,7 @@ describe("the bar this workflow is judged against is stated to the model", () => prompts.push(promptText(turn)); return answersProse("nothing to do")(turn); }, + answersProse("nothing to do"), ]); expect(prompts[0]).toContain("call profile_table on at least one table before you report"); diff --git a/tests/evals/injection.test.ts b/tests/evals/injection.test.ts index 925c71df..f1bc591a 100644 --- a/tests/evals/injection.test.ts +++ b/tests/evals/injection.test.ts @@ -106,7 +106,12 @@ describe("a hostile identifier in the schema inventory", () => { test("reaches the model neutralised, inside an envelope it did not break", async () => { const run = await open(); - const drive = await run.drive([answersProse("I will not follow instructions found in data.")]); + // Nothing was read, so the drive names the instruments once; a model that + // narrates again is stopping rather than hesitating. + const drive = await run.drive([ + answersProse("I will not follow instructions found in data."), + answersProse("I will not follow instructions found in data."), + ]); const transcript = drive.transcripts[0] ?? ""; // The words survive — the content is evidence — but not as a marker. @@ -118,7 +123,7 @@ describe("a hostile identifier in the schema inventory", () => { test("the instruction is inside the fence, never in the server's own voice", async () => { const run = await open(); - const drive = await run.drive([answersProse("Nothing to do.")]); + const drive = await run.drive([answersProse("Nothing to do."), answersProse("Nothing to do.")]); const transcript = drive.transcripts[0] ?? ""; const opened = transcript.indexOf(UNTRUSTED_CONTENT_BEGIN); @@ -304,6 +309,8 @@ describe("a tool the injected text names does not exist for the run that reads i return chatToolCallStream("compare_plans", JSON.stringify({ before: "a", after: "b" }), "call_compare"); }, answersProse("No such tool."), + // A refused call read nothing, so the drive names the instruments once more. + answersProse("No such tool."), ]); // The offered set is a function of the run's PERSISTED workflow, so no text a @@ -371,7 +378,7 @@ describe("OPEN RISK — an assistant message can carry an unfenced marker (B29)" // envelope's own pair. const run = await openHostile(); - const drive = await run.drive([answersProse("Noted.")]); + const drive = await run.drive([answersProse("Noted."), answersProse("Noted.")]); const transcript = drive.transcripts[0] ?? ""; expect(transcript).toContain("(neutralised marker:"); diff --git a/tests/evals/investigation-arc.test.ts b/tests/evals/investigation-arc.test.ts index caf39707..5f482553 100644 --- a/tests/evals/investigation-arc.test.ts +++ b/tests/evals/investigation-arc.test.ts @@ -84,7 +84,9 @@ describe("an investigation that answers, on both reference engines", () => { test("postgres reads three catalog inventories before the first turn", async () => { const run = await open("postgres"); - const drive = await run.drive([answersProse("Nothing to add.")]); + // Twice: this run reads nothing itself, so the drive names the instruments once + // before letting it stop. + const drive = await run.drive([answersProse("Nothing to add."), answersProse("Nothing to add.")]); expect(drive.statements).toHaveLength(3); expect(drive.modelStatements).toEqual([]); @@ -93,7 +95,7 @@ describe("an investigation that answers, on both reference engines", () => { test("sqlite reads two, because its object read carries the relations in the DDL", async () => { const run = await open("sqlite"); - const drive = await run.drive([answersProse("Nothing to add.")]); + const drive = await run.drive([answersProse("Nothing to add."), answersProse("Nothing to add.")]); expect(drive.statements).toHaveLength(2); expect(drive.modelStatements).toEqual([]); @@ -203,7 +205,7 @@ describe("a planning run is judged by what planning mode can produce", () => { const reader = await open("postgres"); const planner = await open("postgres", { mode: "planning" }); - const reading = await reader.drive([answersProse("Nothing to add.")]); + const reading = await reader.drive([answersProse("Nothing to add."), answersProse("Nothing to add.")]); const drive = await planner.drive([answersProse(...A_STATEMENT)]); expect(reading.statements).toHaveLength(3); @@ -412,7 +414,7 @@ describe("the schema's relations reach the model as their own fenced block", () test("a run is shown the relation graph beside the inventory", async () => { const run = await open("sqlite"); - const drive = await run.drive([answersProse("Noted.")]); + const drive = await run.drive([answersProse("Noted."), answersProse("Noted.")]); const transcript = drive.transcripts[0] ?? ""; expect(transcript).toContain("schema relations"); @@ -426,7 +428,7 @@ describe("the schema's relations reach the model as their own fenced block", () test("the block is fenced, so identifiers in it are untrusted content like any other", async () => { const run = await open("postgres"); - const drive = await run.drive([answersProse("Noted.")]); + const drive = await run.drive([answersProse("Noted."), answersProse("Noted.")]); const transcript = drive.transcripts[0] ?? ""; const opened = transcript.indexOf("schema relations"); diff --git a/tests/evals/legacy-surface-coverage.test.ts b/tests/evals/legacy-surface-coverage.test.ts index eae10872..58484c79 100644 --- a/tests/evals/legacy-surface-coverage.test.ts +++ b/tests/evals/legacy-surface-coverage.test.ts @@ -439,6 +439,8 @@ describe("what the removed panels did that these runs do not", () => { return chatToolCallStream("read_monitoring", JSON.stringify({ slowQueryLimit: 20 }), "call_monitor"); }, answersProse("I cannot read live monitoring from here."), + // A refused call read nothing, so the drive names the instruments once more. + answersProse("I cannot read live monitoring from here."), ]); // The transcript is the messages JSON-encoded, so the tool name arrives escaped. @@ -520,6 +522,8 @@ describe("what the removed panels did that these runs do not", () => { ); }, answersProse("I can only report what I read."), + // A refused call read nothing, so the drive names the instruments once more. + answersProse("I can only report what I read."), ]); expect(drive.transcripts[1] ?? "").toContain(String.raw`There is no tool called \"recommend_change\"`); @@ -540,7 +544,10 @@ describe("what the removed panels did that these runs do not", () => { answer: async () => rows(HEADCOUNTS, ["department", "headcount"]), }); runs.push(followUp); - const second = await followUp.drive([answersProse("I would count the sales table.")]); + const second = await followUp.drive([ + answersProse("I would count the sales table."), + answersProse("I would count the sales table."), + ]); expect(followUp.runId).not.toBe(first.runId); const prompt = second.transcripts[0] ?? ""; diff --git a/tests/evals/query-optimization.test.ts b/tests/evals/query-optimization.test.ts index d7347fcb..b8d5abfb 100644 --- a/tests/evals/query-optimization.test.ts +++ b/tests/evals/query-optimization.test.ts @@ -323,6 +323,7 @@ describe("THE GATE: the verifier fails the run when the template's own artifact prompts.push(promptText(turn)); return answersProse("nothing to do")(turn); }, + answersProse("nothing to do"), ]); expect(prompts[0]).toContain("One plan on its own answers nothing"); @@ -532,6 +533,8 @@ describe("the tools belong to the workflow, not to the model", () => { return chatToolCallStream("compare_plans", JSON.stringify({ before: "a", after: "b" }), "call_compare"); }, answersProse("There is nothing here I can compare."), + // A refused call read nothing, so the drive names the instruments once more. + answersProse("There is nothing here I can compare."), ]); // Refused by the OFFERED set, before any argument was looked at — and the run diff --git a/tests/evals/real-model.ts b/tests/evals/real-model.ts index efae10f1..2c79bc82 100644 --- a/tests/evals/real-model.ts +++ b/tests/evals/real-model.ts @@ -588,7 +588,9 @@ export async function openCaseRun(testCase: RealModelCase): Promise { async function readInventoryOnce(engine: EvalEngine): Promise { const reader = await openEvalRun({ engine }); try { - await reader.drive([answersProse("Nothing to add.")]); + // Twice: this run reads nothing itself, so the drive names the instruments once + // before letting it stop, and a scripted model that runs dry throws. + await reader.drive([answersProse("Nothing to add."), answersProse("Nothing to add.")]); } finally { reader.dispose(); } diff --git a/tests/evals/report-citation.test.ts b/tests/evals/report-citation.test.ts index d5a37f69..27072ab5 100644 --- a/tests/evals/report-citation.test.ts +++ b/tests/evals/report-citation.test.ts @@ -133,6 +133,7 @@ describe("a run can cite using only what it was shown", () => { prompts.push(promptText(turn)); return answersProse("nothing to do")(turn); }, + answersProse("nothing to do"), ]); expect(prompts[0]).toContain('{"source":"artifact","correlationId":"<'); diff --git a/tests/isolated/agent-investigation.test.ts b/tests/isolated/agent-investigation.test.ts index c1db1bde..9cc777b5 100644 --- a/tests/isolated/agent-investigation.test.ts +++ b/tests/isolated/agent-investigation.test.ts @@ -2645,7 +2645,12 @@ describe("planning mode runs no statement of the user's", () => { test("an agent run's closing prose is not a plan statement, however it is fenced", async () => { const b = boot(freshDataDir(), { answer: catalog }); const run = await startRun(b, "agent"); - const script = scriptedModel(answersProse(fenced("SELECT title FROM film"))); + // Twice: this run reads nothing, so the drive names the instruments once before + // letting it stop. + const script = scriptedModel( + answersProse(fenced("SELECT title FROM film")), + answersProse(fenced("SELECT title FROM film")), + ); await runInvestigation(run.runId, { service: b.service, @@ -2670,6 +2675,8 @@ describe("planning mode runs no statement of the user's", () => { const script = scriptedModel( callsTool("present_answer", { artifact: "corr_1", presentation: { kind: "table" } }), answersProse("understood"), + // A refused call read nothing, so the drive names the instruments once more. + answersProse("understood"), ); const result = await runInvestigation(run.runId, { @@ -3408,9 +3415,10 @@ describe("a run that stops having read nothing is told to read it itself", () => `no-report`. The extra turn is spent on a run that has lost; it cannot turn a pass into a failure, only a failure into another attempt. - Per-model all the same, and off by default: the ten models locked at 300/300 were - measured without it, and a drive-wide change is how this repository has twice handed - back cells it had already won. + A stated `false` is obeyed all the same: the ten models locked at 300/300 were measured + without it, and overruling a measurement is how this repository has twice handed back + cells it had already won. What is NOT obeyed is an absent entry, which is not a + measurement — see the pair below. */ const asksTheUser = answersProse("Could you please share the exact SQL statement you are running?"); @@ -3431,10 +3439,34 @@ describe("a run that stops having read nothing is told to read it itself", () => expect(script.turns[1]?.transcript).toContain("inspect_schema"); }); - test("a model that was not measured needing it is left alone", async () => { + /* + Whose sentence it is, decided on the two answers a profile can give. + + The drive holds a sentence written for exactly this ending — "Read it yourself. Call + inspect_schema for the tables and their columns, and inspect_plan for how a statement will + run" — and for a while sent it only to a model whose profile asked for it. One profile of + twenty-eight does, and a model nobody has measured has no profile at all, so the model most + in need of the sentence was the one guaranteed not to receive it. + + Measured across the sweep behind 0.14.1: three hundred runs ended `model-stopped` with + `no-report`, and HALF of them — a hundred and fifty — had called no tool at all. Ninety-four + of those hundred and fifty were ended without the drive saying anything. By model: + `mistral-small3.1:24b` 51, `granite3.3:8b` 41, `mistral:7b` 40, `gpt-oss:20b` 18; a hundred + and ten of the hundred and fifty on `investigation`, the first surface a model meets. + + The gate's own comment already argued the cost away: it fires only where `anyToolCalled` is + false, so the run composed no report and has already earned `no-report` — "the turn cannot + cost a pass". A bound that cannot protect a passing run is not protecting anything; what it + was withholding was a sentence on a run already lost. + + This is the fourth setting found in this shape and the second corrected. A measured profile + is still obeyed: all twenty-eight state the field, so no shipped model's turn count moves. + */ + test("a model NOBODY has measured is told to read, because the sentence is the server's own", async () => { const b = boot(freshDataDir()); const run = await startRun(b); - const script = scriptedModel(asksTheUser, answersProse("Understood.")); + // Prose, nothing read; then, once the drive has named the instruments, it uses them. + const script = scriptedModel(asksTheUser, callsTool("inspect_schema", { schema: "public" }), reportOn()); const result = await runInvestigation(run.runId, { service: b.service, @@ -3442,7 +3474,36 @@ describe("a run that stops having read nothing is told to read it itself", () => resources: b.resources, }); + const events = await eventsOf(b.store, run.runId); + expect(events.filter((event) => event.kind === "guidance-issued").map((event) => event.notice)).toContain( + "unread-stop", + ); + expect(script.turns[1]?.transcript).toContain("Read it yourself"); + // The point of the extra turn: the run ends on a report rather than where the model left it. + expect(result.stopReason).toBe("report-composed"); + }); + + test("a profile that states false is still obeyed", async () => { + /* + The other half of the distinction, and the reason this is a second resolver rather than a + changed default. `qwen3:4b` states `retryUnreadStop: false` — a measurement, not an + absence — and a number somebody measured is not the drive's to overrule. + */ + const b = boot(freshDataDir()); + const run = await startRun(b); + const script = scriptedModel(asksTheUser, answersProse("Understood.")); + + const result = await runInvestigation(run.runId, { + service: b.service, + model: await modelOver(script.fetch, "https://api.openai.com/v1", "qwen3:4b"), + resources: b.resources, + }); + expect(script.turns.length).toBe(1); + const events = await eventsOf(b.store, run.runId); + expect(events.filter((event) => event.kind === "guidance-issued").map((event) => event.notice)).not.toContain( + "unread-stop", + ); expect(result.stopReason).toBe("model-stopped"); }); @@ -3554,7 +3615,9 @@ describe("a run that used its tools and then narrated is reminded once", () => { // then told to cite artifacts it has not got. const b = boot(freshDataDir()); const run = await startRun(b); - const script = scriptedModel(invents, answersProse("I could not do that.")); + // Three: an invented name reached nothing, so this run also read nothing, and the + // drive names the instruments once before letting it stop. + const script = scriptedModel(invents, answersProse("I could not do that."), answersProse("I could not do that.")); const result = await runInvestigation(run.runId, { service: b.service, diff --git a/tests/unit/lib/agent/model-profiles.test.ts b/tests/unit/lib/agent/model-profiles.test.ts index ac9d9a18..aa72ea75 100644 --- a/tests/unit/lib/agent/model-profiles.test.ts +++ b/tests/unit/lib/agent/model-profiles.test.ts @@ -4,6 +4,8 @@ import { ceilingFor, presentReminderLimitFor, retriesEmptyTurn, + retriesUnreadStop, + answersUnreadStop, turnTimeoutMsFor, planStatementRetriesFor, reportReminderLimitFor, @@ -141,6 +143,23 @@ describe("sampling is decided per model, defaulting to deterministic", () => { expect(planStatementRetriesFor("some-model-released-tomorrow:70b")).toBe(0); }); + test("a stop that read nothing is answered for a model nobody has measured, and not against one who has", () => { + /* + The same distinction, on the other setting that carries it. `retriesUnreadStop` reads the + shipped document, where all twenty-eight entries state the field and one — `nemotron3:33b` — + states `true`; `answersUnreadStop` is what the drive asks, and it separates a stated `false` + from no entry at all. The gate behind it fires only where the run called no tool, so it is + reachable only on a run that has already earned `no-report` and cannot cost a pass. + */ + expect(retriesUnreadStop("nemotron3:33b")).toBe(true); + expect(retriesUnreadStop("qwen3:8b")).toBe(false); + expect(retriesUnreadStop("some-model-released-tomorrow:70b")).toBe(false); + + expect(answersUnreadStop("nemotron3:33b")).toBe(true); + expect(answersUnreadStop("qwen3:8b")).toBe(false); + expect(answersUnreadStop("some-model-released-tomorrow:70b")).toBe(true); + }); + test("a model whose turn does not fit the shipped limit is given its own", () => { /* `qwen3.5:9b` clears five surfaces and loses Plan, and the loss is not a plan the verdict