From e970f10cca5beab649b70d9398da2e13bb001580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Bene=C5=A1?= Date: Fri, 7 Aug 2026 09:13:54 +0200 Subject: [PATCH] A time in no particular zone, and a preview missing what it previews The first audit pass: everything from 0.8.1 to 0.9.8 read as a whole rather than one feature at a time, starting with what a model is actually told. Four of these had shipped as correct. The date line carried a timezone variable that resolves to nothing until somebody chooses one -- so every default account was told times were "in unless they say otherwise", while two comments asserted the line disappeared instead. The prompt preview built its variables without a chat, which is what eleven fragments are gated on, so the whole agent surface was absent from it whatever was ticked. Plan mode was instructed to keep its plan current with a tool that mode withdraws. And knowledge_get returned a document whole where every sibling reader caps and says so, its description promising exactly that. The subagent guidance was wrong in both directions at once: it denied a documented parameter and named seven of twenty-three allowed commands. Both halves are pinned by tests against the real list and the real schema now, because prose and a constant drift the moment one is edited alone. docs/notes/audit-0.9.md carries the findings that are not fixed here, with why -- the ones whose fix would change what a feature does are the user's call, not this pass's. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 36 ++++++++ src/lembas/__init__.py | 2 +- src/lembas/api/admin_prompts.py | 96 +++++++++++++++++++-- src/lembas/services/harness.py | 33 ++++++- src/lembas/services/prompts.py | 75 ++++++++++------ src/lembas/services/subagent.py | 19 ++-- src/lembas/services/tools.py | 44 +++++++++- src/lembas/web/templates/admin/prompts.html | 27 ++++++ tests/test_admin_prompts.py | 60 +++++++++++++ tests/test_agent_tools.py | 61 +++++++++++++ tests/test_harness.py | 31 +++++++ tests/test_subagent.py | 38 ++++++++ tests/test_tools.py | 50 +++++++++++ 13 files changed, 521 insertions(+), 51 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c857b62..54e45a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,42 @@ for 1.0.0 have something to be assembled from. ## Unreleased +## 0.9.9 + +**The first of five audit passes before 1.0.0** — everything that landed between +0.8.1 and 0.9.8 read as a whole rather than one feature at a time. This one is +the main logic, the harness, and every instruction a model is given. + +- Fixed: **every model was told the time in a zone with no name.** On any + account that had not chosen a timezone — which is the default state of every + account — the date line shipped as "Times the person gives you are in + unless they say otherwise", on every request. The code claimed in two places + that the line disappeared instead. It never had. +- Fixed: **the prompt preview could not show most of what it previews.** Eleven + fragments are gated on things that only exist once there is a real chat, and + the preview has none — so the whole agent surface, both scheduling fragments + and the helper warning were missing from it whatever you ticked. Editing + `tool.agent` and pressing preview showed a system message without `tool.agent` + in it, and nothing said so. Two new controls come with the fix: what kind of + chat to preview as, and which agent mode. +- Fixed: **a model in Plan mode was told to use a tool it did not have.** + `plan_update` is withdrawn in that mode in favour of `plan_submit`, but its + guidance appeared whenever a plan existed — directly under the line saying + anything not in your tool list does not exist. +- Fixed: **reading one knowledge document could fill the whole context window.** + Every other reader caps what it returns and says so; this one returned the + document whole, and its description said "in full", so it did exactly what it + claimed. A long PDF is now cut at 40,000 characters with the model told. +- Fixed: **the guidance about helpers on a machine was wrong in both + directions.** It denied that a helper can write files, which is a documented + option of the tool beside it, and it named seven of the twenty-three commands + a helper may run — so a model avoided commands it was allowed to use. Both are + now checked against the real list and the real schema by tests, because prose + and a constant drift the moment one is edited alone. +- The tool description for delegating no longer claims a helper gets "the same + tools". It gets deliberately fewer, and sizing a task against the wrong set is + how a whole phase gets planned around something that will refuse it. + - The Updates page notices when the update helper on a host was installed for a **different channel** than the page follows. It is declared in two places — `lembas.env` and the systemd unit — and only the installer writes both, so diff --git a/src/lembas/__init__.py b/src/lembas/__init__.py index 1cf1196..e3c56de 100644 --- a/src/lembas/__init__.py +++ b/src/lembas/__init__.py @@ -1,3 +1,3 @@ """LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints.""" -__version__ = "0.9.8" +__version__ = "0.9.9" diff --git a/src/lembas/api/admin_prompts.py b/src/lembas/api/admin_prompts.py index 8816629..76880d4 100644 --- a/src/lembas/api/admin_prompts.py +++ b/src/lembas/api/admin_prompts.py @@ -18,6 +18,7 @@ from lembas.services import harness as harness_service from lembas.services import prompts as prompts_service from lembas.services import settings_store from lembas.services import tools as tools_service +from lembas.services.agent import policy from lembas.web.templating import render log = logging.getLogger(__name__) @@ -28,6 +29,49 @@ router = APIRouter(prefix="/admin/prompts", tags=["admin-prompts"]) # in place rather than imagined. An administrator can clear the field. SAMPLE_DOCUMENTS = "report.pdf, notes.txt" +# The rest of what a preview has to pretend, and the reason it must. +# +# `harness.context_variables` fills most `requires` gates only when it is handed +# a real `Chat` -- the machine, the directory, the plan, the project listing, a +# scheduled task's instruction, the flag saying this is a helper. The preview +# passes `chat=None`, so every one of those stayed empty and **eleven gated +# fragments could never appear in it at all**: the whole agent surface, both +# scheduling fragments, and the helper warning. An administrator editing +# `tool.agent` previewed a system message with `tool.agent` missing from it, and +# nothing said so. +# +# Samples rather than a transient Chat. `compose_from` takes plain variables +# precisely so this screen never has to build one, and a constructed row would +# need a connection, a profile and a directory that exist -- inventing an SSH +# host to render a paragraph is a worse trade than inventing the paragraph's +# values. This is what `SAMPLE_DOCUMENTS` has always done, extended to the rest. +SAMPLE_AGENT = { + "agent_target": "buildbox", + "agent_dir": "/srv/www/example", + "agent_rewound": "on 3 August at 14:20", + "background": "on", + "project_files": "src/\n app.py\n models.py\nREADME.md\npyproject.toml", + "agent_instructions": "Run the tests with `make check` before proposing a change.", + "agent_instructions_file": "AGENTS.md", + "plan": "1. [done] Read the failing test\n2. [doing] Fix the parser\n3. [todo] Add a case", +} + +SAMPLE_SCHEDULE = { + "schedule_instruction": "Summarise what changed in the repository since yesterday.", + "schedule_summary": "every weekday at 08:00", +} + +# Situations a chat can be in that are not a tool family, so nothing on the +# "Tools offered" row can reach them. `kind` and `parent_chat_id` in the model. +SITUATION_ORDINARY = "" +SITUATION_TASK = "task" +SITUATION_HELPER = "helper" +SITUATIONS = ( + (SITUATION_ORDINARY, "An ordinary chat"), + (SITUATION_TASK, "A scheduled task, running unattended"), + (SITUATION_HELPER, "A helper sent by another model"), +) + def _families_of(db: Db, names: list[str]) -> list[str]: """Keep only real family names, in the registry's order. @@ -54,6 +98,8 @@ def _variables( model_name: str = "", bases: str = "", documents: str = "", + situation: str = SITUATION_ORDINARY, + mode: str = "", ) -> dict[str, str]: """The preview's variable values. @@ -64,7 +110,14 @@ def _variables( No Chat row is made. `harness.compose_from` takes plain variables precisely so that this screen never has to build a transient one. + + The samples are gated exactly as `context_variables` gates the real values -- + the agent block on the `agent` family, the schedule and helper blocks on the + situation rather than on any family, because neither is a tool. A preview + that admitted a fragment the real request would not is worse than one that + omitted it, so the gating is mirrored rather than approximated. """ + from lembas.services.agent import policy from lembas.services.library import memories as memories_service from lembas.services.library import skills as skills_service @@ -79,6 +132,18 @@ def _variables( "document_names": documents, } ) + if "agent" in families: + values.update(SAMPLE_AGENT) + # A real one out of the table, not invented prose: this bullet *is* the + # mode guidance, so a made-up sentence here would preview wording that + # no request ever carries. + values["agent_mode"] = policy.MODE_GUIDANCE.get(mode, "") or policy.MODE_GUIDANCE[ + policy.MODE_EDIT + ] + if situation == SITUATION_TASK: + values.update(SAMPLE_SCHEDULE) + if situation == SITUATION_HELPER: + values["subagent"] = "yes" return values @@ -109,16 +174,27 @@ async def prompts_page(request: Request, db: Db, user: AdminUser, saved: bool = "variables": prompts_service.VARIABLES, # The legend shows what each name resolves to right now, with every # family on -- a legend nobody can check is just a list of words. - "resolved": _variables( - db, - user, - families=families, - model_name=models[0].label if models else "", - bases="Contracts, Recipes", - documents=SAMPLE_DOCUMENTS, - ), + # Every situation at once, unlike the preview: a chat is either a + # scheduled task or a helper and never both, but a legend is a + # reference rather than a rendering, and a name shown as empty + # because of the situation it was built in reads as a name that + # resolves to nothing. + "resolved": { + **_variables( + db, + user, + families=families, + model_name=models[0].label if models else "", + bases="Contracts, Recipes", + documents=SAMPLE_DOCUMENTS, + situation=SITUATION_TASK, + ), + "subagent": "yes", + }, "models": models, "families": families, + "situations": SITUATIONS, + "modes": policy.MODE_LABELS, "registry": sorted( tools_service.registry(db).values(), key=lambda t: (t.family, t.name) ), @@ -173,6 +249,8 @@ async def preview(request: Request, db: Db, user: AdminUser): model_name = str(form.get("preview_model") or "") bases = str(form.get("preview_bases") or "").strip() documents = str(form.get("preview_documents") or "").strip() + situation = str(form.get("preview_situation") or "") + mode = str(form.get("preview_mode") or "") variables = _variables( db, @@ -181,6 +259,8 @@ async def preview(request: Request, db: Db, user: AdminUser): model_name=model_name, bases=bases, documents=documents, + situation=situation, + mode=mode, ) body = harness_service.compose_from( db, diff --git a/src/lembas/services/harness.py b/src/lembas/services/harness.py index 819101f..4389331 100644 --- a/src/lembas/services/harness.py +++ b/src/lembas/services/harness.py @@ -188,10 +188,23 @@ def context_variables( "today": stamp.strftime("%A %-d %B %Y"), "now": stamp.strftime("%A %-d %B %Y, %H:%M (UTC%z)"), # Named so a model working out a schedule can say which zone it meant, - # and so `core.today` can carry it without a second fragment. Empty when - # nobody has chosen one, which drops the line rather than printing the - # server's zone as though it were a decision. - "timezone": clock.name_for(user), + # and so `core.today` can carry it without a second fragment. + # + # The fallback is load-bearing and used to be absent. `name_for` returns + # "" for anybody who has never chosen a zone -- the default state of + # every account -- and the comment here claimed that dropped the line + # rather than announcing the server's zone as a decision. It did not: + # `substitute` drops a line only when it is *blank* after expansion, and + # this variable sits inside a sentence, so every such request shipped + # "- Times the person gives you are in unless they say otherwise." + # + # Naming the server's zone was never the thing being avoided anyway. + # `stamp` is `clock.now_for(user)`, which already falls back to it, so + # `{{today}}` and `{{now}}` are *already* in that zone and `{{now}}` + # already prints its offset. Withholding the label from a value the + # model has been given is not restraint, it is a hole. This is the + # fallback `schedule/compile.py` has always had, for the same reason. + "timezone": clock.name_for(user) or str(clock.server_zone()), "instance_name": branding.for_db(db).name, "user_name": (user.name or "") if user is not None else "", "model_name": "", @@ -241,6 +254,7 @@ def context_variables( "agent_instructions": "", "agent_instructions_file": "", "plan": "", + "plan_editable": "", # Empty everywhere but a scheduled task's own chat, which is what makes # it the gate on `core.unattended` as well as the content of # `context.schedule`. Two fragments, one variable, and no way for the @@ -346,6 +360,17 @@ def _agent_values(db: DBSession, chat, user) -> dict[str, str]: # `agent_session.resolve`. A plan the model cannot see is a plan it # cannot keep current, which is the whole of why this is here. "plan": plans_service.render_block(context.plan), + # Whether `plan_update` is actually in this request, which is not the + # same question as whether there is a plan. `agent/tools.py` drops it in + # Plan mode -- that mode ends with `plan_submit` instead -- so gating its + # guidance on `plan` alone told a model in Plan mode to "keep it current + # with plan_update as you go" about a tool that was not there, directly + # under `core.tool_list` saying anything unnamed does not exist. The + # fragment's own hint claimed the two coincided. They do not, and this + # is the variable that makes them. + "plan_editable": ( + plans_service.render_block(context.plan) if context.mode != policy.MODE_PLAN else "" + ), **_project_instructions(db, chat, context, settings_store), } diff --git a/src/lembas/services/prompts.py b/src/lembas/services/prompts.py index ecfcc17..09c47c6 100644 --- a/src/lembas/services/prompts.py +++ b/src/lembas/services/prompts.py @@ -219,8 +219,14 @@ VARIABLES: tuple[Variable, ...] = ( "The current plan", "The plan this agent chat is working to, with its ids, finished phases " "collapsed and the active one shown in full. Empty when there is none, " - "which is what keeps both the plan section and plan_update's guidance " - "out of every chat that is not carrying one.", + "which is what keeps the plan section out of every chat not carrying one.", + ), + Variable( + "plan_editable", + "The plan, when it can be changed", + "The same text as {{plan}}, but blank in Plan mode -- where plan_update " + "is withdrawn and the turn ends with plan_submit instead. Gates that " + "tool's guidance, so a mode without the tool is not told to use it.", ), Variable( "agent_instructions", @@ -614,9 +620,10 @@ BUILTIN: tuple[Fragment, ...] = ( order=20, variables=("today", "timezone"), hint="A model has no clock. Without this it cannot tell whether what it " - "recalls is current, and will not think to check. The timezone line " - "carries its own variable, so it disappears on an instance where nobody " - "has chosen one rather than announcing the server's as a decision.", + "recalls is current, and will not think to check. {{timezone}} is the " + "reader's own zone, or the server's where they have not chosen one -- " + "the same zone the date above is already stated in, so it names a value " + "rather than making a decision.", default=( "Today is {{today}}. Your training data stops well before this, so treat " "anything time-sensitive as something to check rather than something you " @@ -1314,20 +1321,28 @@ BUILTIN: tuple[Fragment, ...] = ( families=("subagent",), requires=("agent_target",), hint="The agent-chat half, gated on `agent_target` so it appears only " - "where there is a machine. What it has to say is what a helper cannot " - "do there, because the failure otherwise is a model planning a whole " - "phase around a helper that will refuse every step of it — a helper " - "reads and may run a short list of read-only commands, and nothing " - "else, whatever mode this chat is in.", + "where there is a machine. What it has to say is where the edge is, " + "because the failure otherwise is a model planning a whole phase around " + "a helper that will refuse every step of it. Two edges, and they are " + "different: **commands** are pinned to a read-only list in every mode, " + "since an unattended chat cannot approve anything; **files** may be " + "written, but only by a helper asked for with write and only from Edit " + "or Auto. An earlier version denied the second outright — which is a " + "documented parameter of the tool beside it — and named seven of the " + "twenty-three allowed commands, so a model avoided commands it had.", default=( "- A helper on this machine reads and reports. It can list and read files " - "and run the ordinary read-only commands — ls, cat, grep, find, git status, " - "git log, git diff — and nothing else, in every mode, because there is " - "nobody there to approve anything. Send one to find out where something " - "lives, to read a subsystem and describe it, or to check whether a pattern " - "holds across a tree; make the changes yourself once it reports.\n" - "- Do not send one to build, test, install or run anything: it will be " - "refused a step in and come back having done nothing.\n" + "and run the read-only commands — ls, pwd, cat, head, tail, wc, file, stat, " + "du, df, tree, find, grep, rg, and git status, log, show, diff, branch and " + "remote. Send one to find out where something lives, to read a subsystem " + "and describe it, or to check whether a pattern holds across a tree.\n" + "- Do not send one to build, test, install or run anything. That list is " + "the whole of what it may run, in every mode, because there is nobody " + "there to approve anything else — a helper asked to run the tests is " + "refused a step in and comes back having done nothing.\n" + "- A helper asked for with write can also write and edit files, and only " + "from Edit or Auto mode. Use it for a mechanical change across many files " + "that you have already decided on; keep the deciding for yourself.\n" "- Ask for what you want back, not for a summary. “The three files that " "define X and what each does” is usable; “look into X” comes back as prose " "you have to read the codebase to check." @@ -1539,12 +1554,14 @@ BUILTIN: tuple[Fragment, ...] = ( group=GROUP_TOOLS, order=255, families=("agent",), - requires=("plan",), - hint="Appears once a plan exists, which is also when plan_update is " - "offered. It is about doing the bookkeeping as the work goes rather " - "than at the end -- a plan updated only at the end is a report, and " - "the point of it is being able to see where things are while they are " - "still moving.", + requires=("plan_editable",), + hint="Appears when plan_update is actually offered: a plan exists and " + "this is not Plan mode, which withdraws it in favour of plan_submit. " + "Gated on {{plan}} it appeared in Plan mode too, telling a model to " + "use a tool it did not have. It is about doing the bookkeeping as the " + "work goes rather than at the end -- a plan updated only at the end is " + "a report, and the point of it is being able to see where things are " + "while they are still moving.", default=( "- There is a plan for this work, set out below. Keep it current with " "plan_update as you go rather than at the end: mark a task “doing” when " @@ -1567,16 +1584,18 @@ BUILTIN: tuple[Fragment, ...] = ( requires=("plan",), variables=("plan",), hint="The plan as it stands, including what has already been ticked " - "off. A plan the model cannot see is a plan it cannot update, which " - "is what the whole of plan_update depends on. The ids are shown " - "because they are what plan_update takes.", + "off. A plan the model cannot see is a plan it cannot work to. Shown in " + "every mode including Plan, where the tool for changing it is withdrawn " + "-- so this says what the plan *is* and leaves how to change it to " + "`tool.plan_update`, which is gated on that tool actually being there. " + "The ids are shown because they are what plan_update takes.", default=( "### The current plan\n" "\n" "{{plan}}\n" "\n" - "This is the plan as it stands now. Change it with plan_update rather " - "than restating it in your answer, and quote the ids above." + "This is the plan as it stands now. Work to it, and quote the ids above " + "rather than restating the plan in your answer." ), ), Fragment( diff --git a/src/lembas/services/subagent.py b/src/lembas/services/subagent.py index 4f5080d..8c031c1 100644 --- a/src/lembas/services/subagent.py +++ b/src/lembas/services/subagent.py @@ -516,14 +516,17 @@ def tool_defs() -> list[ToolDef]: family=FAMILY_SUBAGENT, description=( "Hand one self-contained piece of work to a helper — a second " - "model with the same tools that works on its own and gives you " - "its answer. Use it to cover several independent areas at once: " - "call it several times in one turn and each runs in parallel. " - "It cannot ask you or the reader anything, cannot delegate " - "further, and starts knowing nothing about this conversation, " - "so the task must say everything it needs. Do not use it for " - "something you could do in one call yourself, or for anything " - "needing a decision only the reader can make." + "model that works on its own and gives you its answer. Use it " + "to cover several independent areas at once: call it several " + "times in one turn and each runs in parallel. It gets a " + "narrower set of tools than you: it reads, it cannot ask you or " + "the reader anything, it cannot delegate further, it changes " + "nothing unless you set write, and on a machine it may run only " + "a fixed list of read-only commands. It starts knowing nothing " + "about this conversation, so the task must say everything it " + "needs. Do not use it for something you could do in one call " + "yourself, or for anything needing a decision only the reader " + "can make." ), parameters={ "type": "object", diff --git a/src/lembas/services/tools.py b/src/lembas/services/tools.py index 7feb2de..2adf164 100644 --- a/src/lembas/services/tools.py +++ b/src/lembas/services/tools.py @@ -188,6 +188,21 @@ RISKS = (RISK_READ, RISK_WRITE, RISK_EXECUTE, RISK_ASK) # single page. Cut with the model told so, rather than refused. MAX_FETCH_CHARS = 20_000 +# The same bound for a knowledge document, and it was missing. `knowledge_get` +# returned `extracted_text` whole while every sibling reader capped and said so +# -- `fetch` above, `file_read`, the memories block, the skill index, the +# project listing. `MAX_EXTRACTED_CHARS` is 120_000 by default and an +# administrator can raise it, so one call on a long PDF filled an ordinary +# window with nothing anywhere reporting that it had. +# +# Larger than a fetched page on purpose. Somebody put this document in the +# library deliberately and named it in a search; a page the model followed a +# link to is a guess. Cut with the model told so rather than refused, which is +# what `fetch` and `file_read` both do -- a reader that fails on exactly the +# documents worth reading is worse than one that hands back the first part and +# says there was more. +MAX_DOCUMENT_CHARS = 40_000 + @dataclass class ToolContext: @@ -471,6 +486,14 @@ async def _run_knowledge_get(context: ToolContext, args: dict[str, Any]) -> Tool "results": [{"title": document.title, "id": document.id}], } body = document.extracted_text or document.extraction_error or "(no text)" + if len(body) > MAX_DOCUMENT_CHARS: + body = ( + f"{body[:MAX_DOCUMENT_CHARS]}\n\n" + f"[Cut off here. This document is {len(document.extracted_text or ''):,} " + f"characters and the first {MAX_DOCUMENT_CHARS:,} are above. Search it " + "with knowledge_search to find the part you need.]" + ) + event["truncated"] = True return ToolOutcome(f"{document.title}\n\n{body}", event) @@ -912,8 +935,21 @@ async def _run_ask_user(context: ToolContext, args: dict[str, Any]) -> ToolOutco session-free snapshot that deliberately holds no way to reach one. Getting here means some other path called `run_tool` directly, and saying so is better than returning an empty answer the model would treat as a reply. + + It read `args["question"]`, singular, against a schema that declares + `questions` and a list -- so the event it built always carried an empty + `query`, and the card showed a refusal with no sign of what had been asked. + Harmless only because this path is unreachable, which is exactly why nothing + caught it: schema drift on a branch no test exercises. Tolerant of the same + spellings `generation._questions_in` accepts, rather than importing it, + which would be a circular import for one field on a dead path. """ - question = str(args.get("question") or "").strip() + asked: Any = args.get("questions") or args.get("question") or "" + if isinstance(asked, list): + asked = asked[0] if asked else "" + if isinstance(asked, dict): + asked = asked.get("question") or "" + question = str(asked).strip() return ToolOutcome( "That question could not be put to anyone, so it has gone unanswered. " "Carry on without it, or say what you need.", @@ -997,7 +1033,11 @@ REGISTRY: dict[str, ToolDef] = { ToolDef( name="knowledge_get", family=FAMILY_KNOWLEDGE, - description="Read one knowledge document in full, by the id a search returned.", + description=( + "Read one knowledge document, by the id a search returned. A " + "long one is cut off at the end rather than refused, and you " + "are told when that happened." + ), parameters=_object({"id": _STRING}, ["id"]), run=_run_knowledge_get, ), diff --git a/src/lembas/web/templates/admin/prompts.html b/src/lembas/web/templates/admin/prompts.html index c0c82f0..b13c7d7 100644 --- a/src/lembas/web/templates/admin/prompts.html +++ b/src/lembas/web/templates/admin/prompts.html @@ -82,6 +82,33 @@ + {# + The two axes a tool family cannot reach. Without them every fragment + gated on a chat -- the whole agent surface, both scheduling fragments and + the helper warning -- was missing from the preview whatever was ticked + below, because the preview passes no Chat and those variables are only + filled when there is one. + #} +
+ + +
+
+ + +

+ Only reaches the prompt with agent ticked below. The + machine, directory and plan shown there are samples. +

+
Tools offered {% for family in families %} diff --git a/tests/test_admin_prompts.py b/tests/test_admin_prompts.py index 7636efc..8d819e8 100644 --- a/tests/test_admin_prompts.py +++ b/tests/test_admin_prompts.py @@ -34,6 +34,21 @@ def test_the_page_lists_every_fragment(client: TestClient, registered): assert f'name="prompt.{fragment.key}"' in page, fragment.key +def test_the_preview_controls_are_inside_what_the_preview_includes( + client: TestClient, registered +): + """`hx-include="#prompt-form, #preview-controls"` is the whole wiring, so a + control placed outside that container is submitted by nothing and changes + nothing -- with no error, which is this codebase's recurring failure. Assert + the containment rather than the markup of any one field. + """ + page = client.get("/admin/prompts").text + controls = page.split('id="preview-controls"', 1)[1].split("
\n ", 1)[0] + for name in ("preview_model", "preview_bases", "preview_documents", + "preview_situation", "preview_mode", "preview_family"): + assert f'name="{name}"' in controls, name + + def test_the_page_is_refused_to_a_plain_user(client: TestClient, plain_user): assert client.get("/admin/prompts").status_code == 403 assert client.post("/admin/prompts", data={}).status_code == 403 @@ -135,6 +150,51 @@ def test_the_preview_escapes_what_a_model_wrote(client: TestClient, db, register assert "<img src=x" in body +def test_the_preview_can_reach_the_fragments_that_need_a_chat(client: TestClient, registered): + """Eleven fragments are gated on variables `context_variables` fills only + when it is handed a real `Chat`, and the preview hands it `None`. So the + entire agent surface, both scheduling fragments and the helper warning were + absent from every preview whatever was ticked -- an administrator editing + `tool.agent` previewed a system message with `tool.agent` missing from it, + and nothing said so. The samples are what close that. + """ + agent = client.post( + "/admin/prompts/preview", + data={"preview_family": ["agent"], "preview_mode": "plan"}, + ).text + # tool.agent (agent_target), and the mode picked rather than a fixed one. + assert "buildbox" in agent + assert "Plan** mode" in agent + # tool.project_files, context.agent_instructions, context.plan, tool.background + assert "pyproject.toml" in agent + assert "AGENTS.md" in agent + assert "Fix the parser" in agent + + task = client.post( + "/admin/prompts/preview", data={"preview_situation": "task", "preview_family": []} + ).text + assert "every weekday at 08:00" in task + + helper = client.post( + "/admin/prompts/preview", data={"preview_situation": "helper", "preview_family": []} + ).text + assert "buildbox" not in helper + assert helper != task + + +def test_the_preview_gates_its_samples_exactly_as_a_real_request_would( + client: TestClient, registered +): + """A preview that admitted a fragment the real request would not is worse + than one that omitted it, so the samples follow the same gates: the agent + block on the family, the other two on the situation and on no family at all. + """ + plain = client.post("/admin/prompts/preview", data={"preview_family": ["notes"]}).text + assert "buildbox" not in plain + assert "AGENTS.md" not in plain + assert "every weekday at 08:00" not in plain + + def test_the_preview_warns_when_the_cap_would_cut_it_off(client: TestClient, db, registered): settings_store.update(db, {"max_harness_chars": 60}, key=settings_store.PROMPTS) body = client.post("/admin/prompts/preview", data={"preview_family": ["web_search"]}).text diff --git a/tests/test_agent_tools.py b/tests/test_agent_tools.py index 20e28f3..9a8e245 100644 --- a/tests/test_agent_tools.py +++ b/tests/test_agent_tools.py @@ -986,6 +986,67 @@ async def test_an_ordinary_chat_is_not_asked_to_narrate(db, user_id, machine): assert "Settle what you are setting out to achieve" not in text +def _give_a_plan(db, chat): + """A plan on the chat, the way `_plan_of` finds one: a message carrying it + and the chat pointing at that message.""" + from lembas.services import plans + + message = Message( + chat_id=chat.id, + role=ROLE_ASSISTANT, + content="", + plan_json=plans.build(title="Fix the parser", steps="Read it\nChange it"), + ) + db.add(message) + db.commit() + chat.plan_message_id = message.id + db.commit() + + +async def test_plan_mode_is_not_told_to_use_a_tool_it_does_not_have(db, user_id, machine): + """`agent/tools.py` withdraws `plan_update` in Plan mode -- that mode ends + with `plan_submit` instead. Its guidance was gated on {{plan}}, which is set + whenever a plan exists in any mode, so a model in Plan mode was told to + "keep it current with plan_update as you go" about a tool that was not in + its list, directly under `core.tool_list` saying anything unnamed does not + exist. The fragment's own hint asserted the two coincided. + """ + from lembas.services import harness + + chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_PLAN) + _give_a_plan(db, chat) + user = db.get(User, user_id) + + offered = tools_service.resolve_tools(db, chat, user) + assert "plan_update" not in offered.by_name + + values = harness.context_variables(db, user, offered.schemas, chat) + # The plan is still shown -- a model that cannot see it cannot submit a + # better one. Only the instruction to *edit* it goes. + assert values["plan"] + assert values["plan_editable"] == "" + + text = harness.compose(db, user, offered.schemas, chat) + assert "Fix the parser" in text + assert "plan_update" not in text + + +async def test_a_mode_that_has_plan_update_is_still_told_about_it(db, user_id, machine): + """The other half, or the gate is just a deletion.""" + from lembas.services import harness + + chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_EDIT) + _give_a_plan(db, chat) + user = db.get(User, user_id) + + offered = tools_service.resolve_tools(db, chat, user) + assert "plan_update" in offered.by_name + + values = harness.context_variables(db, user, offered.schemas, chat) + assert values["plan_editable"] == values["plan"] != "" + assert "plan_update" in harness.compose(db, user, offered.schemas, chat) + + async def test_an_agent_chat_is_told_its_real_round_budget(db, user_id, machine): """MAX_ROUNDS is one. An agent chat gets hundreds, and telling it one would be a false fact about its own budget on every turn.""" diff --git a/tests/test_harness.py b/tests/test_harness.py index 2f20236..1ce954a 100644 --- a/tests/test_harness.py +++ b/tests/test_harness.py @@ -37,6 +37,37 @@ def test_no_tools_means_no_tool_guidance(db, owner): assert harness.compose(db, owner, None) == text +def test_the_date_line_names_a_zone_when_nobody_has_chosen_one(db, owner): + """The shipped default, on the shipped configuration. + + `clock.name_for` returns "" for an account that has never chosen a zone, + which is every account until somebody visits the settings page. The variable + sits *inside* a sentence, and `substitute` drops a line only when the whole + line is blank after expansion -- so an unresolved zone was never a dropped + line, it was "in unless", shipped on every request. Both the comment in + `harness.py` and the fragment's own hint claimed otherwise, and no test + asked. + """ + assert not (owner.settings_json or {}).get("timezone") + text = harness.compose(db, owner, []) + + line = next(ln for ln in text.splitlines() if "Times the person gives you" in ln) + assert "in unless" not in line + assert " " not in line + # Whatever the host is set to, it has to be *something*. + zone = line.split(" are in ", 1)[1].split(" unless", 1)[0] + assert zone.strip() + + +def test_the_date_line_uses_the_readers_own_zone_when_they_have_one(db, owner): + """And a chosen zone is preferred to the server's, which is the whole point + of the variable -- the fallback must not have flattened it.""" + owner.settings_json = {**(owner.settings_json or {}), "timezone": "Pacific/Auckland"} + db.commit() + text = harness.compose(db, owner, []) + assert "are in Pacific/Auckland unless" in text + + def test_clearing_the_core_fragments_restores_an_empty_harness(db, owner): """The behaviour change is a default, not a rule: an administrator who wants nothing sent to a tool-less model can still have exactly that.""" diff --git a/tests/test_subagent.py b/tests/test_subagent.py index c758902..599dc4d 100644 --- a/tests/test_subagent.py +++ b/tests/test_subagent.py @@ -672,3 +672,41 @@ def test_a_helpers_chat_is_sized_by_its_own_numbers(db): assert limits.steps == 7 assert limits.wall_seconds == 111.0 assert agent_session.resolve(db, parent, _user(db)).limits.steps == 200 + + +# --- What the model is told it may do ----------------------------------------- +def test_the_guidance_names_every_command_a_helper_may_run(): + """`SAFE_COMMANDS` holds twenty-three entries; the fragment named seven of + them and said "and nothing else", so a model avoided commands it had -- + which costs nothing visible and is therefore never reported. Pinned rather + than proof-read, because the two drift the moment one is edited alone. + """ + from lembas.services import prompts + + text = next(f for f in prompts.BUILTIN if f.key == "tool.subagent_agent").default + for entry in subagent_service.SAFE_COMMANDS: + if entry.startswith("file_"): + continue # a tool name, not a command -- see SAFE_COMMANDS' comment + # The word a model would look for: the command itself, or for git the + # subcommand, since the prose shares one "git" across all six. + parts = entry.removesuffix("*").strip().split() + word = parts[1] if parts[0] == "git" else parts[0] + assert word in text, f"{entry!r} is allowed and unmentioned" + + +def test_the_guidance_does_not_deny_a_parameter_the_tool_offers(): + """It said a helper "reads and reports ... and nothing else, in every mode", + beside a `write` parameter that makes one write files. A model reads the + prose, not the schema, so the parameter was effectively unreachable. + """ + from lembas.services import prompts + + text = next(f for f in prompts.BUILTIN if f.key == "tool.subagent_agent").default + assert "write" in text + assert "nothing else, in every mode" not in text + + schema = subagent_service.tool_defs()[0] + assert "write" in schema.parameters["properties"] + # And the command list really is fixed whatever `write` says, which is the + # distinction the wording now has to carry. + assert "in every mode" in text diff --git a/tests/test_tools.py b/tests/test_tools.py index e44eb5d..f7c00bc 100644 --- a/tests/test_tools.py +++ b/tests/test_tools.py @@ -415,6 +415,56 @@ async def test_a_failed_fetch_does_not_kill_the_reply(monkeypatch): assert "not reachable" in outcome.content +async def test_a_long_knowledge_document_is_cut_and_the_model_told(db, user_id): + """It was the one reader with no bound at all. `fetch` caps and says so, + `file_read` caps and says so, the memories block, the skill index and the + project listing all carry budgets -- and `knowledge_get` returned + `extracted_text` whole. Its description said "in full", which is why this + read as correct: the tool did exactly what it claimed, and what it claimed + was one call away from filling the window with nothing reporting it. + """ + from lembas.db.models import User + from lembas.services.library import documents as documents_service + + owner = db.get(User, user_id) + base = documents_service.create_base(db, owner=owner, name="Long") + document = documents_service.store_upload( + db, owner=owner, payload=b"x" * 90_000, filename="big.txt", + title="Big", base=base, + ) + + outcome = await tools_service.run_tool( + tools_service.ToolContext(owner_id=user_id), + "knowledge_get", + json.dumps({"id": document.id}), + ) + assert len(outcome.content) < tools_service.MAX_DOCUMENT_CHARS + 500 + assert "Cut off here" in outcome.content + assert outcome.event["truncated"] is True + + +async def test_a_short_knowledge_document_is_returned_whole(db, user_id): + """The cap must not become a truncation notice on ordinary documents.""" + from lembas.db.models import User + from lembas.services.library import documents as documents_service + + owner = db.get(User, user_id) + base = documents_service.create_base(db, owner=owner, name="Short") + document = documents_service.store_upload( + db, owner=owner, payload=b"The mallorn is golden.", filename="a.txt", + title="Mallorn", base=base, + ) + + outcome = await tools_service.run_tool( + tools_service.ToolContext(owner_id=user_id), + "knowledge_get", + json.dumps({"id": document.id}), + ) + assert "The mallorn is golden." in outcome.content + assert "Cut off here" not in outcome.content + assert "truncated" not in outcome.event + + async def test_a_long_page_is_cut_and_the_model_told(monkeypatch): """120_000 characters is roughly thirty thousand tokens. One call would fill an ordinary window and spend an agent chat's whole output budget."""