diff --git a/CLAUDE.md b/CLAUDE.md index 70895d7..0a4f382 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -20,7 +20,7 @@ lembas info # paths + counts, useful when confused lembas secret-key # generate LEMBAS_SECRET_KEY lembas create-admin # create or promote an admin -pytest # 1854 tests, ~2min +pytest # 1872 tests, ~2min # PLAN.md tracks what is and is not built ruff check . # lint (line length 100) python scripts/build_artwork.py # regenerate artwork (SVG + PWA icons; @@ -128,7 +128,8 @@ src/lembas/ schedule/ work that happens because time passed: clock.py (whose "now"), rule.py (the recurrence, pure and total), ticker.py (the loop and the claim), runner.py (firing), - compile.py (plain words into a rule) + compile.py (plain words into a rule), tool.py (the four + a model calls) wake.py starting a reply from outside a request -- one lock discipline, shared by finished jobs and by schedules sharing.py one visibility rule for every library store @@ -178,8 +179,10 @@ touching the code it names -- these are the same notes, not a summary. harness fragments, SSH, background jobs, the file tools and the patch matcher, the project listing and a project's own AGENTS.md, and the terminal panel. - `docs/notes/schedules-and-reports.md` -- claiming a schedule before firing it, - the pure recurrence rule, the wake lock, task chats, Reports, and why an empty - `kind` means both sides of the sidebar switch and never "no filter". + the pure recurrence rule, the wake lock, task chats, Reports, why an empty + `kind` means both sides of the sidebar switch and never "no filter", and the + four scheduling tools a model calls (which did not exist, so a model asked to + schedule something wrote a note and said it had). - `docs/notes/image-generation.md` -- the ComfyUI workflow with holes in it, what substitution walks, the review-and-retry loop, and how a failure reports itself. diff --git a/PLAN.md b/PLAN.md index 939452d..7c543df 100644 --- a/PLAN.md +++ b/PLAN.md @@ -407,14 +407,20 @@ seen working. container on the network ### Phase 1 — the scheduling tools (`0.9.0`) -- [ ] **A model can schedule.** There was no tool for it — the seam was left +- [x] **A model can schedule.** There was no tool for it — the seam was left (`Schedule.origin` has defined `ORIGIN_MODEL` with no writer since scheduling landed) and the tool was never built, so a model asked to "remind me every Monday" wrote a note and said it had. `schedule_create`, `schedule_list`, `schedule_update` and `schedule_cancel` over the same `rule.validate` the form and the compile already share -- [ ] Guidance saying which target a run should reach, and that anything which - happens later or repeatedly is a schedule rather than a note +- [x] **The reply says the timing back in words.** A schedule is invisible until + it fires, so `rule.describe` in the answer is the only moment anybody can + check that Monday was read as Monday +- [x] The Scheduled list badges the ones nobody typed +- [x] Guidance saying which target a run should reach, and that anything which + happens later or repeatedly is a schedule rather than a note — said in + `tool.notes` and `tool.memory` as well, because those are what the model + actually reached for ### Phase 2 — image generation admin (`0.9.1`) - [ ] **Defaults an administrator can set** — steps, cfg, size, sampler, diff --git a/docs/notes/schedules-and-reports.md b/docs/notes/schedules-and-reports.md index b9ffad2..fa7485f 100644 --- a/docs/notes/schedules-and-reports.md +++ b/docs/notes/schedules-and-reports.md @@ -139,3 +139,48 @@ and hiding it means it can never be found again, let alone filed into. And with agent chats switched off there is no switch and no filtering at all, rather than one side of a fork nobody can move: an administrator turning the feature off would otherwise strand whoever last left it on Agents in an empty sidebar. + +## A model can schedule, and could not before + +**There was no scheduling tool, and that was the whole failure.** Asked to +"remind me every Monday at noon", a model looked down its list, found +`notes_create` described as *"something worth having in a later conversation"* +and `memory_add` beginning with the word *Remember*, wrote a note, and said it +had scheduled something. Every screen agreed with it. No amount of prompting +fixes that: the near-misses were the only thing there was to reach for, and +nothing anywhere said scheduling existed. + +The seam had been left open. `Schedule.origin` has defined `ORIGIN_MODEL` since +the feature shipped with **no writer**, and `services/schedules.py` says in its +first line that it holds "what the routes *and the tools* both need". +`services/schedule/tool.py` is what was meant to go through it. + +**One vocabulary, not a second one.** The four tools are a thin layer over what +the form already uses: `rule.validate` is the single total normaliser — the +manual form, the compile step and the tool all hand it the same raw shape — +`schedules.create` writes the row and the task chat together, and +`rule.describe` says what came out in words. A separate dialect for models would +mean two definitions of "every other Tuesday" and one of them going quietly +wrong. The `tool.schedule` fragment is deliberately worded from +`task.schedule_compile`, which has been turning people's words into this same +JSON since the feature shipped. + +**The tool answers with `rule.describe`, never "done".** A schedule is invisible +until it fires, which may be days away, so the sentence in the reply is the only +moment anybody can check that Monday was understood as Monday. The tool hands +the description over and says, in the result text, to quote it. `ORIGIN_MODEL` +goes on the row for the matching reason: the Scheduled list badges the ones +nobody typed, because otherwise a model's decision and the reader's own are the +same row. + +**Gated on `schedule.use`, not on a `tools.schedule` of its own.** A reader who +may set a schedule up by hand may say so to a model instead, and a second +permission beside the first would only ever be answered "the same as that one". +The instance switch is passed into `_family_allowed` the way `images` is, so an +instance with scheduling off offers nothing — a model handed a tool that cannot +work spends a round finding out, which in a one-round reply is the whole reply. + +**`tool.notes` and `tool.memory` both say what they are not for.** They are what +the model actually reached for, so each ends with the line that redirects: +anything that should *happen* at a time is a schedule, and remembering that +something should happen does not make it happen. diff --git a/src/lembas/__init__.py b/src/lembas/__init__.py index 3a17f3b..d905ad1 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.8.3" +__version__ = "0.9.0" diff --git a/src/lembas/services/prompts.py b/src/lembas/services/prompts.py index 4bada37..6df4afe 100644 --- a/src/lembas/services/prompts.py +++ b/src/lembas/services/prompts.py @@ -988,7 +988,9 @@ BUILTIN: tuple[Fragment, ...] = ( "a summary of a long document. Correct one with notes_edit when it turns out " "to be wrong, and remove it with notes_delete when it is no longer true — a " "stale note is worse than no note. Anything short and durable about the " - "person themselves is a memory rather than a note." + "person themselves is a memory rather than a note, and anything that should " + "*happen* at a time — later, tomorrow, every week — is a schedule rather than " + "either, because a note does nothing at the time it describes." ), ), Fragment( @@ -1014,7 +1016,9 @@ BUILTIN: tuple[Fragment, ...] = ( "health details they have not asked you to keep. Anything longer than a " "sentence, or about the work rather than about them, does not belong here. " "When something you remembered turns out to be wrong, remove it with " - "memory_forget, quoting it in full, rather than adding a correction beside it." + "memory_forget, quoting it in full, rather than adding a correction beside it. " + "“Remind me to…” is not a memory: remembering that something should happen " + "does not make it happen, and a schedule does." ), ), Fragment( @@ -1216,6 +1220,48 @@ BUILTIN: tuple[Fragment, ...] = ( "recurring report so this one can say what changed." ), ), + Fragment( + key="tool.schedule", + label="Scheduling", + group=GROUP_TOOLS, + order=248, + families=("schedule",), + variables=("now",), + hint="Appears when the scheduling tools are offered. Most of this is the " + "rule vocabulary, which is also in the tool's own schema — repeated " + "here because the failure it prevents is expensive and silent: a " + "schedule that names the wrong day looks exactly like a working one on " + "every screen, and nobody finds out until it fires. The opening " + "sentence is the one that matters most, and it is here because of what " + "happened without it: asked to schedule something, a model wrote a " + "note, because a note was the nearest thing in its tool list and " + "nothing said scheduling existed.", + default=( + "- You can make things happen later. schedule_create sets up work that runs " + "because time has passed rather than because somebody asked just now — once, " + "or on a repeat. Use it whenever the person says *when*: “in ten minutes”, " + "“every Monday at noon”, “each morning”, “remind me”. Writing a note or a " + "memory instead makes nothing happen at the time; those are read only when " + "somebody goes looking. schedule_list shows what already exists, " + "schedule_update changes one and schedule_cancel stops it.\n" + "- It is {{now}} where this person is, and every time you write is read in " + "their zone. Work “in ten minutes” and “tomorrow at nine” out from that clock " + "rather than guessing.\n" + '- Use "every" for a plain timer and "at" for a calendar: "in ten minutes" is ' + '{"every": {"minutes": 10}} with a start, and "every Monday at noon" is ' + '{"at": {"weekdays": [0], "times": ["12:00"]}}. Monday is 0 — count the days ' + "off rather than guessing, because naming the wrong one still looks like a " + "working schedule.\n" + "- Choose where the result goes. A reminder or a short daily fact goes to " + "messages; something to read and keep goes to a report; work that builds on " + "the previous run stays in its own chat.\n" + "- Write the instruction so it stands alone. It is read days later by a model " + "that was not here, with nobody to ask what you meant.\n" + "- Say the resulting timing back in your reply — the tool gives it to you in " + "words. That sentence is the only chance the person has to notice a mistake " + "before the first run." + ), + ), Fragment( key="context.knowledge_scope", label="Which knowledge bases", diff --git a/src/lembas/services/schedule/tool.py b/src/lembas/services/schedule/tool.py new file mode 100644 index 0000000..b1960a3 --- /dev/null +++ b/src/lembas/services/schedule/tool.py @@ -0,0 +1,398 @@ +"""Scheduling, as something a model can do rather than only a person. + +## Why this exists + +It did not, and that was the bug. Asked to "remind me every Monday at noon", a +model looked down its tool list, found `notes_create` described as *"something +worth having in a later conversation"* and `memory_add` beginning with the word +*Remember*, wrote a note, and reported that it had scheduled something. Nothing +in the list said scheduling existed; nothing said it did not. The near-misses +were the only thing to reach for. + +No prompt fixes that. The seam had been left open on purpose -- +`Schedule.origin` has defined `ORIGIN_MODEL` since the feature landed, with no +writer, and `services/schedules.py` says in its first line that it holds "what +the routes **and the tools** both need" -- and this is the tool that was meant +to go through it. + +## One vocabulary, not a second one + +Everything below is a thin layer over what the form already uses: +`rule.validate` is the single total normaliser (the manual form, the compile +step and this all hand it the same raw shape), `schedules.create` writes the row +and the task chat together, and `rule.describe` says what came out in words. A +second dialect for models would mean two definitions of "every other Tuesday" +and one of them going quietly wrong. + +The rule shape is documented in `services/schedule/rule.py` and repeated to the +model in the `tool.schedule` fragment, which is deliberately worded from +`task.schedule_compile` -- the prompt that has been turning people's words into +this same JSON since the feature shipped. + +## What the tool answers with + +`rule.describe(...)`, always, not "done". A schedule is invisible until it fires, +which may be days away, so the one moment anybody can check that Monday was +understood as Monday is the sentence in the reply. The model is told to quote it. +`ORIGIN_MODEL` goes on the row for the same reason: the Scheduled list can then +say which of these nobody typed. + +## What it deliberately cannot do + +Touch anybody else's schedules -- `schedules.get` takes the user and returns +None for a row that is not theirs, which is the whole authorisation here as it +is in the routes. And it cannot create an agent task: `schedules.create` refuses +that on its own terms, for reasons written down there. +""" + +from __future__ import annotations + +import logging +from typing import TYPE_CHECKING, Any + +from lembas.db.models import ORIGIN_MODEL, TARGET_CHAT, TARGETS, Schedule, User +from lembas.db.session import session_scope +from lembas.services.schedule import clock +from lembas.services.schedule import rule as rule_service + +if TYPE_CHECKING: # pragma: no cover - typing only + from lembas.services.tools import ToolContext, ToolDef, ToolOutcome + +log = logging.getLogger(__name__) + +# What a schedule may be told to do, as the model sees it. The same three the +# form offers, in the same order, with the wording that says which to pick -- +# `target` is the field a model gets wrong most often, because all three are +# plausible readings of "tell me". +TARGET_HELP = ( + "Where the result goes. " + '"report" for something to read later — an investigation, a summary, a ' + "digest — which is filed in Reports and cannot be replied to. " + '"messages" for a short note to the reader in their ongoing conversation, ' + "which is where a reminder or a fact of the day belongs. " + '"chat" to leave it in the task\'s own chat, which is right when the runs ' + "build on each other and you want the transcript." +) + +RULE_HELP = ( + "When it runs, as an object. Keys, all optional:\n" + '- "start": ISO timestamp for the first (or only) run. Required for a ' + 'one-off, and for "every".\n' + '- "every": a plain timer — one of {"minutes": n}, {"hours": n}, ' + '{"days": n}, {"weeks": n}. Use this for "in ten minutes" and "every six ' + 'hours". Minimum one minute.\n' + '- "at": a calendar — {"weekdays": [...], "days": [1-31], ' + '"months": [1-12], "times": ["HH:MM"]}. Leave a list out to mean every one ' + 'of them. Use this for "every Monday at noon" and "daily at nine". ' + "Weekdays are Monday=0, Tuesday=1, Wednesday=2, Thursday=3, Friday=4, " + "Saturday=5, Sunday=6 — count them off rather than guessing, because " + "naming the wrong day still looks like a working schedule.\n" + '- "count": how many times in total, if a number was given.\n' + '- "until": ISO timestamp to stop after, if one was given.\n' + 'Give both "every" and "at" only for something like "every other Tuesday". ' + "Times are in the reader's own timezone, which the system prompt states." +) + + +def _outcome(text: str, event: dict[str, Any]) -> ToolOutcome: + """Imported here rather than at module scope: `services/tools.py` imports + this module to build the definitions, so a top-level import back is a cycle. + """ + from lembas.services.tools import ToolOutcome + + return ToolOutcome(text, event) + + +def _error(name: str, message: str) -> ToolOutcome: + return _outcome(message, {"name": name, "status": "error", "error": message}) + + +def _summary(schedule: Schedule, owner: User | None) -> str: + return rule_service.describe(schedule.rule_json or {}, zone=clock.zone_for(owner)) + + +def _row(schedule: Schedule, owner: User | None) -> dict[str, Any]: + """One schedule as the transcript shows it, and as the model reads it back.""" + return { + "id": schedule.id, + "title": schedule.title, + "summary": _summary(schedule, owner), + "target": schedule.target, + "enabled": bool(schedule.enabled), + "next": schedule.next_fire_at.isoformat() if schedule.next_fire_at else "", + } + + +# --- Making one ---------------------------------------------------------------- +async def _run_create(context: ToolContext, args: dict[str, Any]) -> ToolOutcome: + from lembas.services import schedules as schedules_service + + title = str(args.get("title") or "").strip() + instruction = str(args.get("instruction") or "").strip() + target = str(args.get("target") or TARGET_CHAT).strip().lower() + raw_rule = args.get("schedule") or args.get("rule") or {} + + if not instruction: + return _error( + "schedule_create", + "A schedule needs an instruction: what should be done each time it " + "runs, written out in full. It will be read on its own, days later, " + "by a model that was not here — so say everything it needs.", + ) + if not isinstance(raw_rule, dict): + return _error("schedule_create", "The schedule must be an object saying when to run.") + if target not in TARGETS: + target = TARGET_CHAT + + with session_scope() as db: + owner = db.get(User, context.owner_id) + if owner is None: # pragma: no cover - a session outliving its user + return _error("schedule_create", "That account no longer exists.") + try: + schedule = schedules_service.create( + db, + owner=owner, + title=title or instruction[:60], + instruction=instruction, + rule=raw_rule, + target=target, + model_id=context.model_id or "", + # The value that has been declared and unwritten since the + # feature shipped. It is what lets the Scheduled list say which + # of these nobody typed. + origin=ORIGIN_MODEL, + ) + except schedules_service.ScheduleError as exc: + # Its message is written for a person and reads correctly to a + # model too -- it says what is wrong with the timing rather than + # that something failed, which is what makes the next attempt + # different from this one. + return _error("schedule_create", str(exc)) + + summary = _summary(schedule, owner) + log.info("%s scheduled %r by model (%s)", owner.email, schedule.title, summary) + return _outcome( + # The summary first, and phrased so quoting it is the obvious thing + # to do: it is the only chance anybody has to notice that Monday was + # read as Monday before the first run arrives. + f"Scheduled {schedule.title!r}: {summary}. " + f"The result goes to {schedule.target}. Tell the reader this summary " + f"in your reply, in your own words, so they can correct it now rather " + f"than when it first runs.", + { + "name": "schedule_create", + "query": schedule.title, + "status": "ok", + "detail": summary, + "results": [_row(schedule, owner)], + }, + ) + + +# --- Reading them back --------------------------------------------------------- +async def _run_list(context: ToolContext, args: dict[str, Any]) -> ToolOutcome: + from lembas.services import schedules as schedules_service + + with session_scope() as db: + owner = db.get(User, context.owner_id) + # `visible` is the same narrowing the routes use, and is the whole + # authorisation: it answers with nothing at all for a missing user. + rows = list(db.scalars(schedules_service.visible(owner).order_by(Schedule.created_at))) + if not rows: + return _outcome( + "There are no schedules.", + {"name": "schedule_list", "status": "ok", "results": []}, + ) + listed = [_row(row, owner) for row in rows] + lines = "\n".join( + f"- {row['id']}: {row['title']} — {row['summary']}, to {row['target']}" + + ("" if row["enabled"] else " (paused)") + for row in listed + ) + return _outcome( + f"{len(listed)} schedule(s):\n{lines}", + {"name": "schedule_list", "status": "ok", "results": listed}, + ) + + +# --- Changing and stopping ----------------------------------------------------- +async def _run_update(context: ToolContext, args: dict[str, Any]) -> ToolOutcome: + from lembas.services import schedules as schedules_service + + with session_scope() as db: + owner = db.get(User, context.owner_id) + schedule = schedules_service.get(db, str(args.get("id") or ""), owner) + if schedule is None: + return _error( + "schedule_update", + "There is no such schedule, or it belongs to someone else. " + "schedule_list gives the ids you can use.", + ) + + raw_rule = args.get("schedule") or args.get("rule") + enabled = args.get("enabled") + try: + if raw_rule is not None or args.get("title") or args.get("instruction") is not None: + schedules_service.update( + db, + schedule, + owner=owner, + title=args.get("title"), + instruction=args.get("instruction"), + rule=raw_rule if isinstance(raw_rule, dict) else None, + target=str(args.get("target") or "") or None, + ) + if enabled is not None: + schedules_service.set_enabled(db, schedule, owner=owner, enabled=bool(enabled)) + except schedules_service.ScheduleError as exc: + return _error("schedule_update", str(exc)) + + summary = _summary(schedule, owner) + state = "running" if schedule.enabled else "paused" + return _outcome( + f"Updated {schedule.title!r}: {summary}, {state}. Say the new timing " + f"in your reply.", + { + "name": "schedule_update", + "query": schedule.title, + "status": "ok", + "detail": summary, + "results": [_row(schedule, owner)], + }, + ) + + +async def _run_cancel(context: ToolContext, args: dict[str, Any]) -> ToolOutcome: + from lembas.services import schedules as schedules_service + + with session_scope() as db: + owner = db.get(User, context.owner_id) + schedule = schedules_service.get(db, str(args.get("id") or ""), owner) + if schedule is None: + return _error( + "schedule_cancel", + "There is no such schedule, or it belongs to someone else. " + "schedule_list gives the ids you can use.", + ) + title = schedule.title + # The transcript is kept, which is `delete`'s own default and the right + # one: removing a timer must not delete a conversation as a side effect. + schedules_service.delete(db, schedule, keep_chat=True) + log.info("%s cancelled schedule %r by model", owner.email if owner else "?", title) + return _outcome( + f"Stopped {title!r}. It will not run again; what it has already " + f"written is kept.", + {"name": "schedule_cancel", "query": title, "status": "ok", "results": []}, + ) + + +def tool_defs() -> list[ToolDef]: + """The four, built here so `services/tools.py` need not know the wording.""" + from lembas.services.tools import FAMILY_SCHEDULE, RISK_READ, RISK_WRITE, ToolDef + + return [ + ToolDef( + name="schedule_create", + family=FAMILY_SCHEDULE, + description=( + "Set something up to happen later, or repeatedly, without anybody " + "asking again. Use this whenever the reader says when something " + "should happen — “in ten minutes”, “every Monday at noon”, " + "“daily”, “remind me”. Writing a note or a memory instead does " + "not make anything happen at the time; a note is read only when " + "somebody goes looking for it. Say the resulting timing back to " + "the reader so they can correct it before the first run." + ), + parameters={ + "type": "object", + "properties": { + "title": { + "type": "string", + "description": "A short name for this, five words or fewer.", + }, + "instruction": { + "type": "string", + "description": ( + "What to do each time it runs, written out in full and " + "as an instruction rather than a description. It is " + "read on its own, with none of this conversation " + "around it and nobody to ask, so say everything it " + "needs." + ), + }, + "target": { + "type": "string", + "enum": list(TARGETS), + "description": TARGET_HELP, + }, + "schedule": {"type": "object", "description": RULE_HELP}, + }, + "required": ["instruction", "schedule"], + }, + run=_run_create, + risk=RISK_WRITE, + ), + ToolDef( + name="schedule_list", + family=FAMILY_SCHEDULE, + description=( + "Everything already scheduled, with its id, its timing in words " + "and when it next runs. Worth calling before setting something " + "up, so an existing one is changed rather than duplicated, and " + "before answering a question about what is scheduled." + ), + parameters={"type": "object", "properties": {}}, + run=_run_list, + risk=RISK_READ, + ), + ToolDef( + name="schedule_update", + family=FAMILY_SCHEDULE, + description=( + "Change a schedule: its timing, its instruction, its title, where " + "its result goes, or whether it is paused. Omit a field to leave " + "it alone. Changing the timing restarts the count, because an " + "edited schedule is a new intention. Ids come from schedule_list." + ), + parameters={ + "type": "object", + "properties": { + "id": {"type": "string", "description": "From schedule_list."}, + "title": {"type": "string"}, + "instruction": {"type": "string"}, + "target": {"type": "string", "enum": list(TARGETS)}, + "schedule": {"type": "object", "description": RULE_HELP}, + "enabled": { + "type": "boolean", + "description": ( + "False to pause it, true to resume. Resuming counts " + "from now, so a schedule paused for a month does not " + "come back owing runs." + ), + }, + }, + "required": ["id"], + }, + run=_run_update, + risk=RISK_WRITE, + ), + ToolDef( + name="schedule_cancel", + family=FAMILY_SCHEDULE, + description=( + "Stop a schedule for good and remove it. What it has already " + "written is kept. To stop one temporarily, use schedule_update " + "with enabled false instead." + ), + parameters={ + "type": "object", + "properties": {"id": {"type": "string", "description": "From schedule_list."}}, + "required": ["id"], + }, + run=_run_cancel, + risk=RISK_WRITE, + ), + ] + + +__all__ = ["RULE_HELP", "TARGET_HELP", "tool_defs"] diff --git a/src/lembas/services/tool_labels.py b/src/lembas/services/tool_labels.py index d922520..4736af4 100644 --- a/src/lembas/services/tool_labels.py +++ b/src/lembas/services/tool_labels.py @@ -67,6 +67,11 @@ LABELS: dict[str, str] = { "report_search": "Reports searched", "report_get": "Report read", "image_generate": "Image", + # Work set up to happen later. + "schedule_create": "Scheduled", + "schedule_list": "Schedules read", + "schedule_update": "Schedule changed", + "schedule_cancel": "Schedule stopped", "memory_add": "Memory saved", "memory_forget": "Memory removed", "skill_get": "Skill read", @@ -103,6 +108,10 @@ ICONS: dict[str, str] = { "report_search": "search", "report_get": "file-text", "image_generate": "image", + "schedule_create": "clock", + "schedule_list": "clock", + "schedule_update": "clock", + "schedule_cancel": "stop-circle", "memory_add": "star", "memory_forget": "trash", "skill_get": "sparkle", @@ -144,6 +153,9 @@ ACTIONS: dict[str, str] = { "report_search": "Search reports", "report_get": "Read a report", "image_generate": "Generate an image", + "schedule_create": "Set up a schedule", + "schedule_update": "Change a schedule", + "schedule_cancel": "Stop a schedule", "memory_add": "Remember something", "memory_forget": "Forget something", "skill_get": "Read a skill", @@ -176,6 +188,11 @@ DETAIL_KEYS: dict[str, str] = { # it. Also what makes the box on the card editable: a prompt corrected # before it runs is the commonest useful edit this feature will see. "image_generate": "prompt", + # The instruction, not the timing. An approval card has room for one line, + # and the instruction is the part a person can read and correct -- the + # timing is an object, and the tool answers with it in words afterwards, + # which is where it is actually checkable. + "schedule_create": "instruction", } diff --git a/src/lembas/services/tools.py b/src/lembas/services/tools.py index 7262e8d..64247c3 100644 --- a/src/lembas/services/tools.py +++ b/src/lembas/services/tools.py @@ -130,6 +130,13 @@ FAMILY_IMAGE = "image" # must not hand out the notebook. FAMILY_REPORT = "report" +# Setting work up to happen later, or repeatedly. Its own family and emphatically +# not part of `notes`: the line between them is the whole reason this exists. A +# note is something to find again; a schedule is something that *happens*, and a +# model with only the first reached for it when asked for the second -- wrote the +# note, said it had scheduled something, and nothing anywhere disagreed. +FAMILY_SCHEDULE = "schedule" + # The built-in families, in the order they are offered. FAMILIES = ( FAMILY_SEARCH, @@ -142,6 +149,7 @@ FAMILIES = ( FAMILY_ASK, FAMILY_IMAGE, FAMILY_REPORT, + FAMILY_SCHEDULE, FAMILY_AGENT, ) @@ -1286,7 +1294,13 @@ REGISTRY: dict[str, ToolDef] = { def _family_allowed( - family: str, *, config: dict, capabilities: dict, allowed: dict, images: bool = False + family: str, + *, + config: dict, + capabilities: dict, + allowed: dict, + images: bool = False, + schedules: bool = False, ) -> bool: """Whether one family is on for this chat. @@ -1319,6 +1333,15 @@ def _family_allowed( # the shape `resolve_tools` already refuses for `skill_get` with an # empty library. `settings_store.images_ready` answers all three. return bool(allowed.get("tools.image") and images) + if gate == FAMILY_SCHEDULE: + # `schedule.use` rather than a `tools.schedule` of its own: a reader who + # may set a schedule up by hand may say so to a model instead, and a + # second permission beside the first would only ever be answered "the + # same as that one". `schedules` is the instance switch, passed in for + # the reason `images` is -- an instance with scheduling off must not + # offer this at all, or a model spends a round being told the tool it + # was handed does not work. + return bool(allowed.get("schedule.use") and schedules) if gate in ( FAMILY_CUSTOM, FAMILY_MCP, @@ -1375,6 +1398,20 @@ def _agent_defs(db: DBSession, chat: Chat | None, user: User | None) -> list[Too return agent_tools.tool_defs(context) +def _schedule_defs() -> list[ToolDef]: + """The scheduling tools. + + Not in `REGISTRY` even though they need no rows and no settings to build, + because the module they live in imports `services/tools.py` for `ToolDef` + and the risk constants -- so importing it back at module scope is a cycle. + A function keeps the import inside the call, which is the same shape + `_agent_defs` and `_image_defs` already have. + """ + from lembas.services.schedule import tool as schedule_tool + + return schedule_tool.tool_defs() + + def _image_defs(db: DBSession, values: dict | None = None) -> list[ToolDef]: """The image tool, whose schema carries this instance's own choices. @@ -1417,9 +1454,19 @@ def registry(db: DBSession) -> dict[str, ToolDef]: working on. The same omission cost custom tools their guidance once already. """ from lembas.services.agent import tools as agent_tools + from lembas.services.schedule import tool as schedule_tool return _book( - [*_row_defs(db, None, everything=True), *agent_tools.tool_defs(), *_image_defs(db)] + [ + *_row_defs(db, None, everything=True), + *agent_tools.tool_defs(), + *_image_defs(db), + # Listed here, ungated, or `harness._families` cannot map + # `schedule_create` back to a family and the guidance never + # reaches the model. That omission has cost two features their + # instructions already. + *schedule_tool.tool_defs(), + ] ) @@ -1446,6 +1493,7 @@ def resolve_tools(db: DBSession, chat: Chat, user: User | None) -> ToolSet: config = settings_store.search(db) image_values = settings_store.images(db) images_ready = settings_store.images_ready(db) + schedules_on = bool(settings_store.schedules(db).get("enabled")) # Resolved against what this reader may see, not against everything that # exists: a tool restricted to a group is not offered outside it. The image @@ -1457,6 +1505,7 @@ def resolve_tools(db: DBSession, chat: Chat, user: User | None) -> ToolSet: *_row_defs(db, user), *_agent_defs(db, chat, user), *(_image_defs(db, image_values) if images_ready else []), + *(_schedule_defs() if schedules_on else []), ] ) @@ -1490,6 +1539,7 @@ def resolve_tools(db: DBSession, chat: Chat, user: User | None) -> ToolSet: capabilities=capabilities, allowed=allowed, images=images_ready, + schedules=schedules_on, ) and gate_of(tool.family) not in off # Nothing to read and nothing to improve. Offering `skill_get` with diff --git a/src/lembas/web/templates/schedules/index.html b/src/lembas/web/templates/schedules/index.html index 5f535e4..e981c96 100644 --- a/src/lembas/web/templates/schedules/index.html +++ b/src/lembas/web/templates/schedules/index.html @@ -44,6 +44,11 @@
{% if item.row.target == "report" %}files a report{% endif %} + {# Which of these nobody typed. A schedule is invisible until it fires, so + the list is where a model's decision is checkable at all -- and the + reader needs to be able to tell one apart from their own without + opening it. #} + {% if item.row.origin == "model" %}set up for you{% endif %} {% if not item.row.enabled %}paused{% endif %} Edit
diff --git a/tests/test_schedule_tool.py b/tests/test_schedule_tool.py new file mode 100644 index 0000000..f4cf426 --- /dev/null +++ b/tests/test_schedule_tool.py @@ -0,0 +1,373 @@ +"""Scheduling, as something a model can do. + +The bug this closes is not a broken feature; it is an absent one. There was no +scheduling tool, so a model asked to "remind me every Monday" looked down a list +containing `notes_create` ("worth having in a later conversation") and +`memory_add` ("Remember one short, durable fact"), wrote a note, and said it had +scheduled something. Every screen agreed with it. + +So the tests here are in two halves: the tool does what the form does, through +the same normaliser; and it is *offered* exactly when it can work, because a +model that cannot see it is back to writing notes. +""" + +from __future__ import annotations + +import json + +import pytest +from sqlalchemy import select + +from lembas.db.models import ( + KIND_TASK, + ORIGIN_MODEL, + ROLE_USER, + TARGET_MESSAGES, + Chat, + Connection, + Model, + Schedule, + User, +) +from lembas.services import settings_store +from lembas.services import tools as tools_service +from lembas.services.crypto import encrypt + + +@pytest.fixture(autouse=True) +def scheduling_allowed(db, registered): + """The instance switch on and the permission granted, so these tests are + about the tools rather than about the gates — which have their own test + below, asserting both directions.""" + settings_store.update(db, {"enabled": True}, key=settings_store.SCHEDULES) + settings_store.update(db, {"default_permissions": {"schedule.use": True}}) + connection = Connection( + name="Test", base_url="http://127.0.0.1:1", api_key_encrypted=encrypt("") + ) + db.add(connection) + db.commit() + db.add( + Model( + connection_id=connection.id, + model_id="test-model", + capabilities_json={"tools": True}, + ) + ) + db.commit() + + +def _user(db) -> User: + return db.scalars(select(User).order_by(User.created_at)).first() + + +def _chat(db, user) -> Chat: + chat = Chat(user_id=user.id, title="t", model_id="test-model") + db.add(chat) + db.commit() + return chat + + +def _context(db, user): + """Built through `resolve_tools`, not by hand, and that is the point: what + may be *run* is what was *offered*. `run_tool` consults `context.tools`, so + a context carrying None would fall back to `REGISTRY` -- which holds the + import-time built-ins only and has never held these.""" + resolved = tools_service.resolve_tools(db, _chat(db, user), user) + return tools_service.context_for(db, user, tools=resolved) + + +async def _run(db, name: str, args: dict): + return await tools_service.run_tool(_context(db, _user(db)), name, json.dumps(args)) + + +# --- Creating ------------------------------------------------------------------- +async def test_a_model_can_schedule_a_calendar_run(db, registered): + outcome = await _run( + db, + "schedule_create", + { + "title": "Fun fact", + "instruction": "Send one random fun fact.", + "target": TARGET_MESSAGES, + "schedule": {"at": {"weekdays": [0], "times": ["12:00"]}}, + }, + ) + + schedule = db.scalars(select(Schedule)).one() + assert schedule.title == "Fun fact" + assert schedule.target == TARGET_MESSAGES + assert schedule.rule_json["at"]["weekdays"] == [0] + assert schedule.next_fire_at is not None + # And its chat, made with it -- the first firing may be days away with + # nobody present to make one. + assert db.get(Chat, schedule.chat_id).kind == KIND_TASK + assert outcome.event["status"] == "ok" + + +async def test_a_model_can_schedule_a_timer(db, registered): + """"In ten minutes" is `every` with a start, and it is the shape a model is + likeliest to get wrong -- the first report of this feature failing was + exactly that request.""" + outcome = await _run( + db, + "schedule_create", + { + "instruction": "Say something random.", + "schedule": {"every": {"minutes": 10}, "start": "2099-01-01T00:00:00Z"}, + }, + ) + + schedule = db.scalars(select(Schedule)).one() + assert schedule.rule_json["every"] == {"minutes": 10} + assert outcome.event["status"] == "ok" + + +async def test_the_reply_is_told_the_timing_in_words(db, registered): + """A schedule is invisible until it fires, which may be days away. The one + moment anybody can check that Monday was understood as Monday is the + sentence in the reply, so the tool hands it over and says to quote it.""" + outcome = await _run( + db, + "schedule_create", + { + "instruction": "Check the build.", + "schedule": {"at": {"weekdays": [0], "times": ["12:00"]}}, + }, + ) + + assert "Monday" in outcome.content + assert "12:00" in outcome.content + assert "Monday" in outcome.event["detail"] + # Said plainly enough that a model has no excuse for answering "done". + assert "in your reply" in outcome.content + + +async def test_a_model_made_schedule_says_so_on_the_row(db, registered): + """`ORIGIN_MODEL` has been declared with no writer since the feature + shipped. It is what lets the Scheduled list say which of these nobody + typed.""" + await _run( + db, + "schedule_create", + {"instruction": "x", "schedule": {"at": {"times": ["09:00"]}}}, + ) + assert db.scalars(select(Schedule)).one().origin == ORIGIN_MODEL + + +async def test_a_timing_nothing_could_run_at_is_refused_in_words(db, registered): + """`rule.validate` empties anything it cannot read, and `create` refuses + that rather than writing a schedule with no next run -- which would look + exactly like a working one on every screen it appears on. + + The message matters as much as the refusal: it says what is wrong with the + timing, which is what makes the model's next attempt different from this + one rather than a repeat. + """ + outcome = await _run( + db, "schedule_create", {"instruction": "x", "schedule": {"whenever": "sometimes"}} + ) + + assert outcome.event["status"] == "error" + assert "when" in outcome.content.lower() + assert db.scalars(select(Schedule)).all() == [] + + +async def test_an_instruction_is_required_and_says_why(db, registered): + """It is read days later by a model that was not here, so an empty one is a + schedule that fires and does nothing.""" + outcome = await _run( + db, "schedule_create", {"schedule": {"at": {"times": ["09:00"]}}} + ) + + assert outcome.event["status"] == "error" + assert db.scalars(select(Schedule)).all() == [] + + +# --- Reading, changing, stopping ------------------------------------------------ +async def test_listing_gives_ids_and_timings(db, registered): + await _run( + db, + "schedule_create", + {"title": "Nightly", "instruction": "x", "schedule": {"at": {"times": ["21:00"]}}}, + ) + schedule = db.scalars(select(Schedule)).one() + + outcome = await _run(db, "schedule_list", {}) + + assert schedule.id in outcome.content + assert "Nightly" in outcome.content + assert outcome.event["results"][0]["id"] == schedule.id + + +async def test_listing_nothing_says_so_rather_than_failing(db, registered): + outcome = await _run(db, "schedule_list", {}) + assert outcome.event["status"] == "ok" + assert outcome.event["results"] == [] + + +async def test_changing_the_timing_takes_effect(db, registered): + await _run( + db, "schedule_create", {"instruction": "x", "schedule": {"at": {"times": ["09:00"]}}} + ) + schedule = db.scalars(select(Schedule)).one() + + await _run( + db, + "schedule_update", + {"id": schedule.id, "schedule": {"at": {"weekdays": [4], "times": ["17:00"]}}}, + ) + + db.refresh(schedule) + assert schedule.rule_json["at"]["weekdays"] == [4] + + +async def test_pausing_and_resuming(db, registered): + await _run( + db, "schedule_create", {"instruction": "x", "schedule": {"at": {"times": ["09:00"]}}} + ) + schedule = db.scalars(select(Schedule)).one() + + await _run(db, "schedule_update", {"id": schedule.id, "enabled": False}) + db.refresh(schedule) + assert not schedule.enabled + + await _run(db, "schedule_update", {"id": schedule.id, "enabled": True}) + db.refresh(schedule) + assert schedule.enabled + + +async def test_cancelling_removes_the_schedule_and_keeps_the_transcript(db, registered): + """Removing a timer must not delete a conversation as a side effect, which + is `delete`'s own default and the reason this passes `keep_chat`.""" + await _run( + db, "schedule_create", {"instruction": "x", "schedule": {"at": {"times": ["09:00"]}}} + ) + schedule = db.scalars(select(Schedule)).one() + chat_id = schedule.chat_id + + await _run(db, "schedule_cancel", {"id": schedule.id}) + + assert db.scalars(select(Schedule)).all() == [] + kept = db.get(Chat, chat_id) + assert kept is not None + assert kept.kind != KIND_TASK # reachable again, rather than in no list at all + + +async def test_somebody_elses_schedule_is_invisible(db, registered): + """`schedules.get` takes the user and answers None for a row that is not + theirs. That is the whole authorisation here, as it is in the routes.""" + from lembas.security.passwords import hash_password + + await _run( + db, "schedule_create", {"instruction": "x", "schedule": {"at": {"times": ["09:00"]}}} + ) + schedule = db.scalars(select(Schedule)).one() + + other = User(email="other@example.test", name="Other", password_hash=hash_password("x" * 12)) + db.add(other) + db.commit() + + resolved = tools_service.resolve_tools(db, _chat(db, other), other) + context = tools_service.context_for(db, other, tools=resolved) + listed = await tools_service.run_tool(context, "schedule_list", "{}") + assert listed.event["results"] == [] + + cancelled = await tools_service.run_tool( + context, "schedule_cancel", json.dumps({"id": schedule.id}) + ) + assert cancelled.event["status"] == "error" + assert db.scalars(select(Schedule)).all() != [] + + +# --- Being offered at all ------------------------------------------------------- +def _offered(db, user) -> set[str]: + chat = Chat(user_id=user.id, title="t", model_id="test-model") + db.add(chat) + db.commit() + return {tool.name for tool in tools_service.resolve_tools(db, chat, user).defs} + + +def test_the_tools_are_offered_when_scheduling_is_on(db, registered): + assert "schedule_create" in _offered(db, _user(db)) + + +def test_nothing_is_offered_when_the_instance_has_scheduling_off(db, registered): + """An instance with the feature off must not hand out a tool that would + work — the switch is the administrator's, and a model that could schedule + round it is the switch not existing.""" + settings_store.update(db, {"enabled": False}, key=settings_store.SCHEDULES) + db.commit() + assert "schedule_create" not in _offered(db, _user(db)) + + +def test_nothing_is_offered_without_the_permission(db, registered): + """`schedule.use`, the same one the pages require: a reader who may not set + a schedule up by hand may not have a model do it for them.""" + settings_store.update(db, {"default_permissions": {"schedule.use": False}}) + db.commit() + user = _user(db) + # An admin resolves to every permission, deliberately -- see + # `permissions.resolve`. So the gate can only be tested on somebody who is + # not one, and the first account registered always is. + user.role = ROLE_USER + db.commit() + assert "schedule_create" not in _offered(db, user) + + +def test_the_guidance_reaches_the_model(db, registered): + """`harness._families` maps an offered tool's name back to a family through + `registry(db)`. A tool missing from there is one whose fragment never + appears — which has cost two features their instructions already, so it is + asserted rather than assumed.""" + from lembas.services import harness + + user = _user(db) + chat = Chat(user_id=user.id, title="t", model_id="test-model") + db.add(chat) + db.commit() + + offered = [ + {"function": {"name": tool.name}} + for tool in tools_service.resolve_tools(db, chat, user).defs + ] + text = harness.compose(db, user, offered, chat) + + assert "schedule_create" in text + # And the sentence that stops it reaching for a note instead. + assert "note" in text.lower() + + +def test_notes_and_memory_point_at_scheduling(db, registered): + """The near-miss descriptions are what the model actually reached for, so + both say what they are not for. Pinned on the defaults rather than on the + rendered prompt: an administrator may reword them, and the point is that + the shipped wording says it.""" + from lembas.services import prompts + + by_key = {fragment.key: fragment for fragment in prompts.BUILTIN} + notes = by_key["tool.notes"].default + memory = by_key["tool.memory"].default + + assert "schedule" in notes + assert "schedule" in memory + + +def test_the_list_says_which_ones_nobody_typed(client, db, registered): + """A schedule is invisible until it fires, so the list is where a model's + decision is checkable at all. Without the badge, one it set up and one the + reader wrote are the same row.""" + schedule = Schedule( + user_id=_user(db).id, + title="Set up by a model", + instruction="x", + rule_json={"at": {"times": ["09:00"]}}, + origin=ORIGIN_MODEL, + enabled=True, + ) + db.add(schedule) + db.commit() + + body = client.get("/scheduled").text + + assert "Set up by a model" in body + assert "set up for you" in body