Two selects that never wrote anything, and a queue
The approval card in Auto mode and the missing /effort were one bug. Both
selects hung their hx-patch on an empty sibling form reached by form="…",
and htmx binds a trigger to the annotated element: change fires on the
select and bubbles to its ancestors, which a sibling is not. The live rows
read agent_mode=manual and params_json={} while the browser showed Auto and
Effort: high. policy.py was never involved.
The verb moves onto the control; the empty form stays as value scoping,
which is the half of the CLAUDE.md note that was right. conftest gains
control_named so a test asserts the element carrying the name carries the
verb, rather than asserting the markup that was there throughout.
The composer's highlight was a third instance of the same carelessness in
CSS: .tok-mention is written for the transcript and scoped to nothing, so
the mirror painted its token in accent-coloured monospace over the
textarea's own text. Scoped under .msg; the mirror restates transparency
and font rather than inheriting them, and bleeds by box-shadow.
/effort is now offered before the first prompt and _new_chat reads it.
/index re-walks the project directory on demand, file_write drops the
listing it just invalidated, and the index ladder falls through to SFTP on
a host that refuses exec instead of returning nothing.
A second message during a reply is queued rather than starting a second
concurrent generation: a real Message row with queued set, so it survives a
restart and can be withdrawn. _drain hands one on at the end of a reply,
_inject takes one in at a tool-round boundary so an agent can be steered
mid-task. Stop leaves the queue undelivered. The terminal's Auto toggle
becomes off/copy/send, and send posts straight to the chat without touching
the composer.
@ now offers notes, skills, this chat's attachments and a URL to fetch; a
knowledge base attaches as a reference rather than a copy. copy_document
carries provenance, which was the one attach path that dropped it.
Also fixes an unrelated live bug: the round loop compared against the
global MAX_ROUNDS of 3 while sizing itself from the agent budget of 40, so
agent replies stopped after three rounds and reported forty.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -209,6 +209,32 @@ def mock_http():
|
||||
httpx.AsyncClient = original
|
||||
|
||||
|
||||
def control_named(html: str, name: str) -> dict[str, str]:
|
||||
"""The attributes of the one element carrying `name="…"`.
|
||||
|
||||
Exists so a test can ask "does the control that carries the name also carry
|
||||
the verb?". Two selects in the composer once delegated their `hx-patch` to
|
||||
an empty sibling form through the `form=` attribute, which scopes values but
|
||||
routes no events -- htmx binds a trigger to the annotated element, and
|
||||
`change` reaches ancestors, never siblings. Both controls were decorative
|
||||
for a whole release, and the tests passed the entire time because they
|
||||
asserted the markup that was there rather than the property that mattered.
|
||||
"""
|
||||
from html.parser import HTMLParser
|
||||
|
||||
found: list[dict[str, str]] = []
|
||||
|
||||
class Finder(HTMLParser):
|
||||
def handle_starttag(self, tag: str, attrs: list[tuple[str, str | None]]) -> None:
|
||||
got = {key: (value or "") for key, value in attrs}
|
||||
if got.get("name") == name:
|
||||
found.append(got)
|
||||
|
||||
Finder().feed(html)
|
||||
assert len(found) == 1, f"expected one element named {name!r}, found {len(found)}"
|
||||
return found[0]
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def user_id(db: Session, registered: dict[str, str]) -> str:
|
||||
"""The registered user's id.
|
||||
|
||||
@@ -254,3 +254,78 @@ def test_somebody_elses_connection_is_not_browsable(
|
||||
# 404 and not 403: whether that connection exists is not this endpoint's to
|
||||
# reveal to somebody who does not own it.
|
||||
assert client.get(f"/api/agents/{profile.id}/browse").status_code == 404
|
||||
|
||||
|
||||
# --- Reading the project directory again -------------------------------------
|
||||
def _agent_chat(db, profile):
|
||||
from lembas.db.models import KIND_AGENT, Chat, Connection, Model
|
||||
|
||||
connection = Connection(name="c", base_url="http://127.0.0.1:1", api_key_encrypted="")
|
||||
db.add(connection)
|
||||
db.commit()
|
||||
db.add(Model(connection_id=connection.id, model_id="m", capabilities_json={"tools": True}))
|
||||
db.commit()
|
||||
|
||||
chat = Chat(
|
||||
user_id=profile.owner_id,
|
||||
model_id="m",
|
||||
connection_id=connection.id,
|
||||
kind=KIND_AGENT,
|
||||
ssh_profile_id=profile.id,
|
||||
project_dir=profile.default_dir,
|
||||
)
|
||||
db.add(chat)
|
||||
db.commit()
|
||||
return chat
|
||||
|
||||
|
||||
def test_reindexing_walks_the_tree_again(client: TestClient, db, registered, served_tree):
|
||||
"""The listing is built only when a reply starts and then held for five
|
||||
minutes, so anything done in the terminal panel is invisible to it until
|
||||
then. This is the way to say "look again"."""
|
||||
from lembas.services.agent import index as index_service
|
||||
|
||||
profile = _profile(db, served_tree)
|
||||
chat = _agent_chat(db, profile)
|
||||
|
||||
response = client.post(f"/api/chats/{chat.id}/index")
|
||||
assert response.status_code == 200, response.text
|
||||
body = response.json()
|
||||
|
||||
assert body["ok"] is True
|
||||
assert body["files"] >= 2 # README.md and src/
|
||||
assert index_service.cached(profile.id, served_tree["root"]) is not None
|
||||
|
||||
|
||||
def test_reindexing_a_plain_chat_says_there_is_nothing_to_read(
|
||||
client: TestClient, db, registered, served_tree
|
||||
):
|
||||
from lembas.db.models import Chat, Connection, Model
|
||||
|
||||
connection = Connection(name="c", base_url="http://127.0.0.1:1", api_key_encrypted="")
|
||||
db.add(connection)
|
||||
db.commit()
|
||||
db.add(Model(connection_id=connection.id, model_id="m"))
|
||||
db.commit()
|
||||
chat = Chat(user_id=_profile(db, served_tree).owner_id, model_id="m",
|
||||
connection_id=connection.id)
|
||||
db.add(chat)
|
||||
db.commit()
|
||||
|
||||
assert client.post(f"/api/chats/{chat.id}/index").status_code == 409
|
||||
|
||||
|
||||
def test_reindexing_somebody_elses_chat_is_not_possible(
|
||||
client: TestClient, db, registered, served_tree
|
||||
):
|
||||
profile = _profile(db, served_tree)
|
||||
chat = _agent_chat(db, profile)
|
||||
|
||||
client.post("/auth/logout", follow_redirects=False)
|
||||
client.post(
|
||||
"/auth/register",
|
||||
data={"name": "Sam", "email": "sam@shire.test", "password": "potatoes-po-ta-toes"},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
assert client.post(f"/api/chats/{chat.id}/index").status_code == 404
|
||||
|
||||
@@ -180,6 +180,43 @@ async def test_forgetting_a_connection_drops_its_listings():
|
||||
assert index_service.cached("profile-2", "/work") is not None
|
||||
|
||||
|
||||
async def test_a_host_that_refuses_to_run_commands_still_gets_a_listing():
|
||||
"""SFTP is the rung for exactly this, and the ladder used to skip it.
|
||||
|
||||
An `ExecError` from git or find -- an SFTP-only account, a forced command,
|
||||
a shell that is `/bin/false` -- escaped the loop and was caught outside it,
|
||||
which returned an empty index without ever trying the one method that would
|
||||
have worked.
|
||||
"""
|
||||
|
||||
class _NoExec(_Fake):
|
||||
async def run(self, request):
|
||||
raise ExecError("This account may not run commands.")
|
||||
|
||||
executor = _NoExec(tree={"/work": [RemoteEntry("README.md", False, 5)]})
|
||||
|
||||
found = await index_service.build(executor, "/work")
|
||||
|
||||
assert found.source == "sftp"
|
||||
assert "README.md" in found.paths
|
||||
|
||||
|
||||
async def test_forgetting_one_tree_leaves_the_others():
|
||||
"""What a write invalidates is the directory it wrote into, not the machine.
|
||||
|
||||
Two chats on one box in different trees share nothing but the connection,
|
||||
and dropping both would make every write cost somebody else a walk.
|
||||
"""
|
||||
executor = _Fake(answers={"git ls-files": _ok("a.py\n")})
|
||||
await index_service.ensure(executor, "profile-1", "/work")
|
||||
await index_service.ensure(executor, "profile-1", "/other")
|
||||
|
||||
index_service.forget_dir("profile-1", "/work")
|
||||
|
||||
assert index_service.cached("profile-1", "/work") is None
|
||||
assert index_service.cached("profile-1", "/other") is not None
|
||||
|
||||
|
||||
def test_reading_the_cache_never_does_work():
|
||||
"""`harness` calls this synchronously while assembling the system message,
|
||||
so it must never be the thing that opens a connection."""
|
||||
|
||||
+19
-11
@@ -18,6 +18,8 @@ from lembas.db.models import KIND_AGENT, Chat, Connection, Model, SshProfile, Us
|
||||
from lembas.services import settings_store
|
||||
from lembas.services.agent import policy
|
||||
|
||||
from .conftest import control_named
|
||||
|
||||
|
||||
def _agent_chat(db, *, mode: str = policy.MODE_MANUAL) -> Chat:
|
||||
"""An agent chat pointed at a profile that is never actually connected to.
|
||||
@@ -93,22 +95,28 @@ def test_the_mode_form_uses_a_method_the_route_serves(client: TestClient, db, re
|
||||
assert chat.agent_mode == policy.MODE_MANUAL
|
||||
|
||||
|
||||
def test_the_rendered_form_patches(client: TestClient, db, registered):
|
||||
"""And that the template actually carries it, since that is where it broke.
|
||||
def test_the_mode_select_carries_its_own_verb(client: TestClient, db, registered):
|
||||
"""The request must hang off the element the event fires on.
|
||||
|
||||
Pinned to the mode form by its id rather than to any `hx-patch` on the
|
||||
page: the model picker and the system-prompt box patch the same URL, so a
|
||||
looser assertion would have passed throughout the entire life of the bug.
|
||||
This is the invariant, and the previous version of this test did not check
|
||||
it. `hx-patch` lived on an empty sibling `<form>` that the select pointed at
|
||||
with `form="…"`, which was enough to make the markup look right and enough
|
||||
to make every assertion here pass -- while htmx bound the `change` listener
|
||||
to the form, and `change` fires on the select and bubbles to its *ancestors*
|
||||
only. The mode never once reached the database.
|
||||
|
||||
So: assert on the control, by name, whichever element that turns out to be.
|
||||
"""
|
||||
chat = _agent_chat(db)
|
||||
body = client.get(f"/chat/{chat.id}").text
|
||||
|
||||
assert 'id="agent-mode-form"' in body
|
||||
form = body[body.index('id="agent-mode-form"') :][:200]
|
||||
assert f'hx-patch="/api/chats/{chat.id}"' in form
|
||||
assert "hx-post" not in form
|
||||
# And that the select outside it is actually submitted by it.
|
||||
assert 'form="agent-mode-form"' in body
|
||||
select = control_named(body, "agent_mode")
|
||||
assert select["hx-patch"] == f"/api/chats/{chat.id}"
|
||||
assert "hx-post" not in select
|
||||
# And the empty form still scopes the values, so the PATCH carries this
|
||||
# field alone rather than the whole composer -- `project_dir` in a PATCH is
|
||||
# a 409.
|
||||
assert select["form"] == "agent-mode-form"
|
||||
|
||||
|
||||
def test_the_mode_is_offered_beside_the_composer_not_in_the_topbar(
|
||||
|
||||
@@ -9,6 +9,7 @@ from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json as _json
|
||||
import time
|
||||
|
||||
import pytest
|
||||
|
||||
@@ -205,6 +206,31 @@ async def test_files_are_written_and_read_back(db, user_id, machine, tmp_path):
|
||||
assert "note.txt" in listed.content
|
||||
|
||||
|
||||
async def test_writing_a_file_drops_the_project_listing(db, user_id, machine):
|
||||
"""Otherwise the model is shown a five-minute-old tree that it knows is
|
||||
wrong, and concludes the file it has just created does not exist.
|
||||
|
||||
The TTL is for drift nobody can see coming. This is not that: it is this
|
||||
process changing the tree it has just described.
|
||||
"""
|
||||
from lembas.services.agent import index as index_service
|
||||
|
||||
chat, profile = _setup(db, user_id, machine, mode=policy.MODE_AUTO)
|
||||
user = db.get(User, user_id)
|
||||
resolved = tools_service.resolve_tools(db, chat, user)
|
||||
context = tools_service.context_for(db, user, chat, tools=resolved)
|
||||
|
||||
index_service._CACHE[(profile.id, machine["dir"])] = index_service.ProjectIndex(
|
||||
paths=("stale.txt",), total=1, source="git", built_at=time.monotonic()
|
||||
)
|
||||
|
||||
await tools_service.run_tool(
|
||||
context, "file_write", '{"path": "fresh.txt", "content": "hi"}'
|
||||
)
|
||||
|
||||
assert index_service.cached(profile.id, machine["dir"]) is None
|
||||
|
||||
|
||||
# --- The runner backstop ----------------------------------------------------------
|
||||
async def test_a_runner_refuses_what_the_mode_forbids(db, user_id, machine):
|
||||
"""`_authorise` is the real gate and runs first. This is the belt to that
|
||||
@@ -486,6 +512,140 @@ async def test_an_agent_chat_is_told_its_real_round_budget(db, user_id, machine)
|
||||
assert values["max_rounds"] == "25"
|
||||
|
||||
|
||||
async def test_an_agent_chat_gets_the_rounds_it_was_promised(db, user_id, machine, monkeypatch):
|
||||
"""And is then allowed to use them, which is the half that was missing.
|
||||
|
||||
The budget sizes the loop and names itself in the out-of-rounds message, and
|
||||
the harness above tells the model the same number. But the comparison that
|
||||
ends the loop read the global `MAX_ROUNDS` of three. So an agent chat
|
||||
allowed forty rounds stopped after three and reported that it had taken
|
||||
forty: two wrong answers to "why did it stop", with no way to tell them
|
||||
apart from the outside.
|
||||
"""
|
||||
settings_store.update(db, {"max_steps": 5}, key=settings_store.AGENTS)
|
||||
chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_AUTO)
|
||||
assistant = Message(chat_id=chat.id, role=ROLE_ASSISTANT, content="", complete=False)
|
||||
db.add(assistant)
|
||||
db.commit()
|
||||
|
||||
payloads: list[dict] = []
|
||||
|
||||
async def stream_chat(_endpoint, payload):
|
||||
payloads.append(payload)
|
||||
yield {
|
||||
"choices": [
|
||||
{"delta": {"tool_calls": [{"index": 0, "id": "c1", "function": {
|
||||
"name": "file_list", "arguments": '{"path": "."}'}}]}}
|
||||
]
|
||||
}
|
||||
|
||||
monkeypatch.setattr(generation_service, "stream_chat", stream_chat)
|
||||
|
||||
async def _no_title(*_args, **_kwargs):
|
||||
return ""
|
||||
|
||||
monkeypatch.setattr("lembas.services.chat.generate_title", _no_title)
|
||||
|
||||
generation = generation_service.Generation(chat_id=chat.id, message_id=assistant.id)
|
||||
await generation_service._run(generation)
|
||||
|
||||
# Five rounds that may call tools, then the one that gives up.
|
||||
assert len(payloads) == 6
|
||||
assert "after 5 rounds" in generation.tool_events[-1]["error"]
|
||||
|
||||
|
||||
# --- Interjecting while it works --------------------------------------------------
|
||||
async def test_a_queued_prompt_is_taken_in_between_rounds(db, user_id, machine, monkeypatch):
|
||||
"""The point of queueing in an agent chat: steering work already under way.
|
||||
|
||||
An agent that has just finished one loop and is about to start another is
|
||||
exactly when "actually, do it the other way" is worth having, and making it
|
||||
wait for the whole reply would mean it arrives after the thing it was meant
|
||||
to change.
|
||||
"""
|
||||
from lembas.services import chat as chat_service
|
||||
|
||||
chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_AUTO)
|
||||
message_id = _pending_reply(db, chat)
|
||||
chat_service.create_message(
|
||||
db, chat, "user", "actually, check the other directory first", queued=True
|
||||
)
|
||||
|
||||
payloads: list[dict] = []
|
||||
monkeypatch.setattr(
|
||||
generation_service,
|
||||
"stream_chat",
|
||||
_stub_stream(
|
||||
[[_chunk("file_list", '{"path": "."}')], [_text("Done.")]],
|
||||
payloads,
|
||||
),
|
||||
)
|
||||
|
||||
generation = generation_service.Generation(chat_id=chat.id, message_id=message_id)
|
||||
await generation_service._run(generation)
|
||||
|
||||
# Verbatim, in the user role, with nothing wrapped around it: this genuinely
|
||||
# is the person at the keyboard, and quoting it would teach the model that a
|
||||
# user turn can be a quotation -- the distinction `execute_plan` relies on.
|
||||
assert payloads[1]["messages"][-1] == {
|
||||
"role": "user",
|
||||
"content": "actually, check the other directory first",
|
||||
}
|
||||
|
||||
|
||||
async def test_an_interjection_is_delivered_only_once(db, user_id, machine, monkeypatch):
|
||||
"""Marked delivered before the request goes out, so a crash loses it rather
|
||||
than asking the same thing twice and letting an agent act on it twice."""
|
||||
from lembas.services import chat as chat_service
|
||||
|
||||
chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_AUTO)
|
||||
message_id = _pending_reply(db, chat)
|
||||
waiting = chat_service.create_message(db, chat, "user", "one more thing", queued=True)
|
||||
|
||||
monkeypatch.setattr(
|
||||
generation_service,
|
||||
"stream_chat",
|
||||
_stub_stream(
|
||||
[
|
||||
[_chunk("file_list", '{"path": "."}')],
|
||||
[_chunk("file_list", '{"path": "src"}')],
|
||||
[_text("Done.")],
|
||||
],
|
||||
[],
|
||||
),
|
||||
)
|
||||
|
||||
generation = generation_service.Generation(chat_id=chat.id, message_id=message_id)
|
||||
await generation_service._run(generation)
|
||||
|
||||
db.expire_all()
|
||||
assert db.get(Message, waiting.id).queued is False
|
||||
assert generation.injected_ids == [waiting.id]
|
||||
|
||||
|
||||
async def test_the_reply_sorts_before_the_prompt_it_took_in(db, user_id, machine, monkeypatch):
|
||||
"""Otherwise the next request reads "answer, then the question it answered",
|
||||
and a small model dutifully answers it a second time."""
|
||||
from lembas.services import chat as chat_service
|
||||
|
||||
chat, _profile = _setup(db, user_id, machine, mode=policy.MODE_AUTO)
|
||||
message_id = _pending_reply(db, chat)
|
||||
waiting = chat_service.create_message(db, chat, "user", "and this", queued=True)
|
||||
|
||||
monkeypatch.setattr(
|
||||
generation_service,
|
||||
"stream_chat",
|
||||
_stub_stream([[_chunk("file_list", '{"path": "."}')], [_text("Done.")]], []),
|
||||
)
|
||||
|
||||
generation = generation_service.Generation(chat_id=chat.id, message_id=message_id)
|
||||
await generation_service._run(generation)
|
||||
|
||||
db.expire_all()
|
||||
reply = db.get(Message, message_id)
|
||||
assert reply.created_at > db.get(Message, waiting.id).created_at
|
||||
|
||||
|
||||
# --- Plan mode's artifact ---------------------------------------------------------
|
||||
def test_plan_submit_is_offered_only_in_plan_mode(db, user_id, machine):
|
||||
"""It ends the reply. A model in Auto mode that proposed a plan instead of
|
||||
|
||||
@@ -17,6 +17,8 @@ from sqlalchemy import select
|
||||
from lembas.db.models import Chat, Connection, Model, User
|
||||
from lembas.services import chat as chat_service
|
||||
|
||||
from .conftest import control_named
|
||||
|
||||
|
||||
def _model(db, **capabilities) -> Model:
|
||||
connection = Connection(name="c", base_url="http://127.0.0.1:1", api_key_encrypted="")
|
||||
@@ -162,3 +164,69 @@ def test_a_model_with_no_defaults_starts_a_plain_chat(client: TestClient, db, re
|
||||
client.post("/api/chats/start", data={"content": "hello", "model_id": "m"})
|
||||
|
||||
assert db.scalars(select(Chat)).one().params_json == {}
|
||||
|
||||
|
||||
# --- Choosable before the first prompt ---------------------------------------
|
||||
def test_the_effort_select_carries_its_own_verb(client: TestClient, db, registered):
|
||||
"""The same invariant the mode select needs, for the same reason.
|
||||
|
||||
Both were built on `form="…"` pointing at an empty sibling form holding the
|
||||
`hx-patch`, and both therefore wrote nothing at all: `form=` scopes the
|
||||
values a request carries, it does not route the event that starts one.
|
||||
"""
|
||||
chat = _chat(db)
|
||||
|
||||
select = control_named(client.get(f"/chat/{chat.id}").text, "reasoning_effort")
|
||||
assert select["hx-patch"] == f"/api/chats/{chat.id}"
|
||||
assert select["form"] == "chat-params-form"
|
||||
|
||||
|
||||
def test_the_effort_is_offered_before_there_is_a_chat(client: TestClient, db, registered):
|
||||
"""Otherwise it is a setting you can only reach once it is too late to use.
|
||||
|
||||
On the new-chat screen there is nothing to PATCH, so it is an ordinary field
|
||||
of the composer's form and carries no verb -- `_new_chat` reads it.
|
||||
"""
|
||||
_model(db)
|
||||
|
||||
select = control_named(client.get("/chat").text, "reasoning_effort")
|
||||
assert "hx-patch" not in select
|
||||
assert "form" not in select
|
||||
|
||||
|
||||
def test_starting_a_chat_with_an_effort_stores_it(client: TestClient, db, registered):
|
||||
_model(db)
|
||||
|
||||
client.post(
|
||||
"/api/chats/start",
|
||||
data={"content": "hello", "model_id": "m", "reasoning_effort": "low"},
|
||||
)
|
||||
|
||||
assert db.scalars(select(Chat)).one().params_json["reasoning_effort"] == "low"
|
||||
|
||||
|
||||
def test_an_explicit_effort_beats_the_models_default(client: TestClient, db, registered):
|
||||
"""An inherited value is a starting point, not a ceiling."""
|
||||
model = _model(db)
|
||||
model.params_json = {"reasoning_effort": "high"}
|
||||
db.commit()
|
||||
|
||||
client.post(
|
||||
"/api/chats/start",
|
||||
data={"content": "hello", "model_id": "m", "reasoning_effort": "low"},
|
||||
)
|
||||
|
||||
assert db.scalars(select(Chat)).one().params_json["reasoning_effort"] == "low"
|
||||
|
||||
|
||||
def test_a_nonsense_effort_at_the_start_falls_back(client: TestClient, db, registered):
|
||||
model = _model(db)
|
||||
model.params_json = {"reasoning_effort": "high"}
|
||||
db.commit()
|
||||
|
||||
client.post(
|
||||
"/api/chats/start",
|
||||
data={"content": "hello", "model_id": "m", "reasoning_effort": "extreme"},
|
||||
)
|
||||
|
||||
assert db.scalars(select(Chat)).one().params_json["reasoning_effort"] == "high"
|
||||
|
||||
@@ -152,6 +152,143 @@ def test_a_plain_chat_gets_a_picker_with_no_file_half(client: TestClient, db, re
|
||||
assert "In the project" not in response.text
|
||||
|
||||
|
||||
# --- What `@` offers where there is no machine at all ------------------------
|
||||
def _library(db):
|
||||
"""A note, a skill and a base belonging to the registered reader."""
|
||||
from lembas.db.models import KnowledgeBase, Note, Skill
|
||||
|
||||
owner = db.scalars(select(User)).first()
|
||||
note = Note(owner_id=owner.id, title="Mallorn notes", body="Golden leaves.")
|
||||
skill = Skill(
|
||||
owner_id=owner.id, name="bake-lembas", description="How to bake it", body="Steps."
|
||||
)
|
||||
base = KnowledgeBase(owner_id=owner.id, name="Contracts")
|
||||
db.add_all([note, skill, base])
|
||||
db.commit()
|
||||
return note, skill, base
|
||||
|
||||
|
||||
def test_notes_and_skills_are_offered_in_a_plain_chat(client: TestClient, db, registered):
|
||||
"""A chat with no SSH connection has no project files, which is exactly why
|
||||
the rest of the library has to be reachable there."""
|
||||
_library(db)
|
||||
|
||||
body = client.get("/api/files/mention-picker", params={"q": "mallorn"}).text
|
||||
assert "Mallorn notes" in body
|
||||
|
||||
body = client.get("/api/files/mention-picker", params={"q": "lembas"}).text
|
||||
assert "bake-lembas" in body
|
||||
|
||||
|
||||
def test_a_knowledge_base_is_only_offered_inside_a_chat(
|
||||
client: TestClient, db, registered, make_chat
|
||||
):
|
||||
"""There is nothing to attach it to before a chat exists -- the same reason
|
||||
project files are absent on the new-chat screen."""
|
||||
_library(db)
|
||||
chat_id = make_chat()
|
||||
|
||||
assert "Contracts" not in client.get("/api/files/mention-picker").text
|
||||
assert "Contracts" in client.get(
|
||||
"/api/files/mention-picker", params={"chat_id": chat_id}
|
||||
).text
|
||||
|
||||
|
||||
def test_a_url_is_offered_as_a_page_to_read(client: TestClient, db, registered):
|
||||
body = client.get(
|
||||
"/api/files/mention-picker", params={"q": "https://tolkien.test/mallorn"}
|
||||
).text
|
||||
|
||||
assert "Fetch this page" in body
|
||||
assert "https://tolkien.test/mallorn" in body
|
||||
|
||||
|
||||
def test_a_note_arrives_with_its_text_and_its_name(client: TestClient, db, registered, make_chat):
|
||||
note, _skill, _base = _library(db)
|
||||
chat_id = make_chat()
|
||||
|
||||
response = client.post(
|
||||
"/api/files/from-note", data={"note_id": note.id, "chat_id": chat_id}
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
attachment = db.scalars(select(Attachment)).one()
|
||||
assert "Golden leaves." in attachment.extracted_text
|
||||
# Provenance, for the reason a project file carries it: a model handed four
|
||||
# documents cannot name one back when asked to work on it.
|
||||
assert attachment.source_label == "Note"
|
||||
assert attachment.source_path == "Mallorn notes"
|
||||
|
||||
|
||||
def test_a_skill_can_be_handed_over_directly(client: TestClient, db, registered, make_chat):
|
||||
"""The index is in the harness and `skill_get` fetches on demand -- but only
|
||||
if the model decides to. `@` is the reader saying "use this one"."""
|
||||
_note, skill, _base = _library(db)
|
||||
chat_id = make_chat()
|
||||
|
||||
client.post("/api/files/from-skill", data={"skill_id": skill.id, "chat_id": chat_id})
|
||||
|
||||
attachment = db.scalars(select(Attachment)).one()
|
||||
assert attachment.source_label == "Skill"
|
||||
assert "Steps." in attachment.extracted_text
|
||||
|
||||
|
||||
def test_a_base_is_attached_as_a_reference_not_a_copy(
|
||||
client: TestClient, db, registered, make_chat
|
||||
):
|
||||
"""Scoping, not copying. A folder of contracts in the window would cost the
|
||||
context on every request forever to answer one question."""
|
||||
from lembas.db.models import Chat
|
||||
|
||||
_note, _skill, base = _library(db)
|
||||
chat_id = make_chat()
|
||||
|
||||
response = client.post(f"/api/chats/{chat_id}/bases", data={"base_id": base.id})
|
||||
|
||||
assert response.status_code == 200
|
||||
db.expire_all()
|
||||
assert [b.id for b in db.get(Chat, chat_id).knowledge_bases] == [base.id]
|
||||
# Nothing was copied into the message.
|
||||
assert db.scalars(select(Attachment)).all() == []
|
||||
|
||||
|
||||
def test_attaching_the_same_base_twice_is_not_an_error(
|
||||
client: TestClient, db, registered, make_chat
|
||||
):
|
||||
from lembas.db.models import Chat
|
||||
|
||||
_note, _skill, base = _library(db)
|
||||
chat_id = make_chat()
|
||||
|
||||
client.post(f"/api/chats/{chat_id}/bases", data={"base_id": base.id})
|
||||
client.post(f"/api/chats/{chat_id}/bases", data={"base_id": base.id})
|
||||
|
||||
db.expire_all()
|
||||
assert len(db.get(Chat, chat_id).knowledge_bases) == 1
|
||||
|
||||
|
||||
def test_a_library_document_now_carries_its_provenance(client: TestClient, db, registered):
|
||||
"""This was the one attach path that dropped it, while a project file beside
|
||||
it carried path and machine."""
|
||||
from lembas.services.fetch import Fetched
|
||||
from lembas.services.library import documents as documents_service
|
||||
|
||||
owner = db.scalars(select(User)).first()
|
||||
base = documents_service.default_base(db, owner)
|
||||
document = documents_service.store_page(
|
||||
db,
|
||||
owner=owner,
|
||||
base=base,
|
||||
page=Fetched(url="https://tolkien.test/c", title="The Contract", text="Terms."),
|
||||
)
|
||||
|
||||
client.post("/api/files/from-knowledge", data={"document_id": document.id})
|
||||
|
||||
attachment = db.scalars(select(Attachment)).one()
|
||||
assert attachment.source_path == "The Contract"
|
||||
assert attachment.source_label == base.name
|
||||
|
||||
|
||||
# --- Attaching ---------------------------------------------------------------
|
||||
def test_a_mentioned_file_arrives_with_its_contents(client: TestClient, db, registered, box):
|
||||
profile = _profile(db, box)
|
||||
|
||||
@@ -0,0 +1,393 @@
|
||||
"""Typing while a reply is being written.
|
||||
|
||||
Before this existed, a second message during a stream was simply accepted: it
|
||||
wrote a second assistant placeholder, started a second `Generation`, and left
|
||||
two replies answering the same chat from two different prefixes of it -- with
|
||||
Stop pointing at whichever bubble came first in the document.
|
||||
|
||||
Now it queues. The queue is not an object: it is "the rows in this chat with
|
||||
`queued` set, oldest first". That is the whole reason it survives a restart and
|
||||
the reason `_prune` cannot take it away.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from fastapi.testclient import TestClient
|
||||
from sqlalchemy import select
|
||||
|
||||
from lembas.api.chats import MAX_QUEUED
|
||||
from lembas.db.models import ROLE_ASSISTANT, ROLE_USER, Chat, Connection, Message, Model
|
||||
from lembas.services import chat as chat_service
|
||||
from lembas.services import generation as generation_service
|
||||
from lembas.services.crypto import encrypt
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def empty_registry():
|
||||
yield
|
||||
generation_service._RUNNING.clear()
|
||||
generation_service._TASKS.clear()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def no_upstream(monkeypatch):
|
||||
"""Replace the producer, so the routes can be driven without a server."""
|
||||
started: list = []
|
||||
|
||||
async def _fake_run(generation):
|
||||
started.append(generation)
|
||||
|
||||
monkeypatch.setattr(generation_service, "_run", _fake_run)
|
||||
return started
|
||||
|
||||
|
||||
def _connection(db) -> Connection:
|
||||
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"))
|
||||
db.commit()
|
||||
return connection
|
||||
|
||||
|
||||
def _mid_reply(db, chat_id: str) -> Message:
|
||||
"""A chat with a reply still being written, which is the whole precondition."""
|
||||
db.add(Message(chat_id=chat_id, role=ROLE_USER, content="what is lembas?"))
|
||||
reply = Message(chat_id=chat_id, role=ROLE_ASSISTANT, content="Way", complete=False)
|
||||
db.add(reply)
|
||||
db.commit()
|
||||
return reply
|
||||
|
||||
|
||||
def _messages(db, chat_id: str) -> list[Message]:
|
||||
return list(
|
||||
db.scalars(
|
||||
select(Message).where(Message.chat_id == chat_id).order_by(Message.created_at)
|
||||
)
|
||||
)
|
||||
|
||||
|
||||
# --- The column --------------------------------------------------------------
|
||||
def test_an_ordinary_message_is_not_queued(db, registered, make_chat):
|
||||
"""Every row that predates the column reads the same way, because
|
||||
`sync_schema` adds a NOT NULL boolean with a literal default of 0."""
|
||||
chat_id = make_chat()
|
||||
chat = db.get(Chat, chat_id)
|
||||
|
||||
assert chat_service.create_message(db, chat, ROLE_USER, "hello").queued is False
|
||||
|
||||
|
||||
def test_a_queued_message_is_left_out_of_the_request(db, registered, make_chat):
|
||||
"""It is in the transcript and it is not in the request. That distinction is
|
||||
the entire feature."""
|
||||
chat_id = make_chat()
|
||||
chat = db.get(Chat, chat_id)
|
||||
chat_service.create_message(db, chat, ROLE_USER, "first")
|
||||
chat_service.create_message(db, chat, ROLE_ASSISTANT, "an answer")
|
||||
chat_service.create_message(db, chat, ROLE_USER, "typed while it worked", queued=True)
|
||||
|
||||
sent = chat_service.build_messages(db, chat)
|
||||
|
||||
assert [m["content"] for m in sent] == ["first", "an answer"]
|
||||
|
||||
|
||||
def test_a_delivered_message_is_sent_on_the_next_turn(db, registered, make_chat):
|
||||
chat_id = make_chat()
|
||||
chat = db.get(Chat, chat_id)
|
||||
chat_service.create_message(db, chat, ROLE_USER, "first")
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "second", queued=True)
|
||||
|
||||
waiting.queued = False
|
||||
db.commit()
|
||||
|
||||
assert [m["content"] for m in chat_service.build_messages(db, chat)] == ["first", "second"]
|
||||
|
||||
|
||||
# --- Queueing ----------------------------------------------------------------
|
||||
def test_a_message_sent_during_a_reply_is_queued(
|
||||
client: TestClient, db, registered, make_chat, no_upstream
|
||||
):
|
||||
"""The bug this replaces: a second POST used to start a second generation."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
_mid_reply(db, chat_id)
|
||||
|
||||
response = client.post(f"/api/chats/{chat_id}/messages", data={"content": "and also this"})
|
||||
assert response.status_code == 200
|
||||
|
||||
rows = _messages(db, chat_id)
|
||||
assert rows[-1].content == "and also this"
|
||||
assert rows[-1].queued is True
|
||||
# Exactly one reply in flight, which is the point.
|
||||
assert len([m for m in rows if m.role == ROLE_ASSISTANT and not m.complete]) == 1
|
||||
|
||||
|
||||
def test_a_queued_bubble_never_carries_a_streaming_shell(
|
||||
client: TestClient, db, registered, make_chat, no_upstream
|
||||
):
|
||||
"""`sse-connect` is the only thing that starts a generation, so a queued
|
||||
turn carrying one would be the second concurrent reply all over again.
|
||||
|
||||
Asserted on the body rather than on a row, unusually and deliberately: the
|
||||
attribute *is* the behaviour here.
|
||||
"""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
_mid_reply(db, chat_id)
|
||||
|
||||
body = client.post(f"/api/chats/{chat_id}/messages", data={"content": "later"}).text
|
||||
|
||||
assert "sse-connect" not in body
|
||||
assert "Waiting to be sent" in body
|
||||
|
||||
|
||||
def test_a_message_with_nothing_in_flight_is_sent_as_before(
|
||||
client: TestClient, db, registered, make_chat, no_upstream
|
||||
):
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
|
||||
response = client.post(f"/api/chats/{chat_id}/messages", data={"content": "hello"})
|
||||
|
||||
assert "sse-connect" in response.text
|
||||
rows = _messages(db, chat_id)
|
||||
assert rows[0].queued is False
|
||||
assert rows[1].role == ROLE_ASSISTANT and rows[1].complete is False
|
||||
|
||||
|
||||
def test_the_queue_is_bounded(client: TestClient, db, registered, make_chat, no_upstream):
|
||||
"""A `for` loop in the terminal panel with Auto send on can produce commands
|
||||
far faster than any model answers them."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
_mid_reply(db, chat_id)
|
||||
|
||||
for n in range(MAX_QUEUED):
|
||||
assert client.post(
|
||||
f"/api/chats/{chat_id}/messages", data={"content": f"line {n}"}
|
||||
).status_code == 200
|
||||
|
||||
refused = client.post(f"/api/chats/{chat_id}/messages", data={"content": "one too many"})
|
||||
|
||||
assert refused.status_code == 409
|
||||
assert len([m for m in _messages(db, chat_id) if m.queued]) == MAX_QUEUED
|
||||
|
||||
|
||||
# --- Delivery ----------------------------------------------------------------
|
||||
async def test_a_finished_reply_delivers_the_next_prompt(
|
||||
db, registered, make_chat, no_upstream
|
||||
):
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
reply = _mid_reply(db, chat_id)
|
||||
chat = db.get(Chat, chat_id)
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "next please", queued=True)
|
||||
|
||||
generation = generation_service.Generation(chat_id=chat_id, message_id=reply.id)
|
||||
generation_service._drain(generation)
|
||||
|
||||
db.refresh(waiting)
|
||||
assert waiting.queued is False
|
||||
assert generation.drained is True
|
||||
assert len([m for m in _messages(db, chat_id) if not m.complete]) == 2
|
||||
|
||||
|
||||
async def test_only_one_prompt_is_delivered_at_a_time(db, registered, make_chat, no_upstream):
|
||||
"""Draining the lot would put two consecutive user turns in the next
|
||||
request, which several local chat templates refuse outright."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
reply = _mid_reply(db, chat_id)
|
||||
chat = db.get(Chat, chat_id)
|
||||
first = chat_service.create_message(db, chat, ROLE_USER, "one", queued=True)
|
||||
second = chat_service.create_message(db, chat, ROLE_USER, "two", queued=True)
|
||||
|
||||
generation_service._drain(
|
||||
generation_service.Generation(chat_id=chat_id, message_id=reply.id)
|
||||
)
|
||||
|
||||
db.refresh(first)
|
||||
db.refresh(second)
|
||||
assert first.queued is False
|
||||
assert second.queued is True
|
||||
|
||||
|
||||
async def test_a_stopped_reply_leaves_the_queue_alone(db, registered, make_chat, no_upstream):
|
||||
"""Stop means stop. This is the decision the whole feature was shaped
|
||||
around, and it must never be relaxed into "stop this one and start the
|
||||
next"."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
reply = _mid_reply(db, chat_id)
|
||||
chat = db.get(Chat, chat_id)
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "next please", queued=True)
|
||||
|
||||
generation = generation_service.Generation(chat_id=chat_id, message_id=reply.id)
|
||||
generation.stopped = True
|
||||
generation_service._drain(generation)
|
||||
|
||||
db.refresh(waiting)
|
||||
assert waiting.queued is True
|
||||
assert generation.drained is False
|
||||
|
||||
|
||||
async def test_an_errored_reply_leaves_the_queue_alone(db, registered, make_chat, no_upstream):
|
||||
"""The endpoint has just failed; sending the next prompt into it spends
|
||||
somebody's words to produce a second failure."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
reply = _mid_reply(db, chat_id)
|
||||
chat = db.get(Chat, chat_id)
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "next please", queued=True)
|
||||
|
||||
generation = generation_service.Generation(chat_id=chat_id, message_id=reply.id)
|
||||
generation.error = "The endpoint refused."
|
||||
generation_service._drain(generation)
|
||||
|
||||
db.refresh(waiting)
|
||||
assert waiting.queued is True
|
||||
|
||||
|
||||
async def test_a_superseded_generation_does_not_drain(db, registered, make_chat, no_upstream):
|
||||
"""A regeneration cancels its predecessor, whose `finally:` still runs --
|
||||
the same reason `_persist` refuses. Without this, regenerating would drain
|
||||
the queue as a side effect."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
reply = _mid_reply(db, chat_id)
|
||||
chat = db.get(Chat, chat_id)
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "next please", queued=True)
|
||||
|
||||
abandoned = generation_service.Generation(chat_id=chat_id, message_id=reply.id)
|
||||
generation_service._RUNNING[reply.id] = generation_service.Generation(
|
||||
chat_id=chat_id, message_id=reply.id
|
||||
)
|
||||
generation_service._drain(abandoned)
|
||||
|
||||
db.refresh(waiting)
|
||||
assert waiting.queued is True
|
||||
|
||||
|
||||
# --- Send now and Discard ----------------------------------------------------
|
||||
def test_discarding_removes_the_row(client: TestClient, db, registered, make_chat):
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
chat = db.get(Chat, chat_id)
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "never mind", queued=True)
|
||||
waiting_id = waiting.id
|
||||
|
||||
response = client.post(f"/api/chats/{chat_id}/messages/{waiting_id}/discard")
|
||||
|
||||
assert response.status_code == 200
|
||||
# Queried rather than `db.get`: the route committed in a session of its own,
|
||||
# and this one still holds the instance.
|
||||
db.expire_all()
|
||||
assert db.scalar(select(Message).where(Message.id == waiting_id)) is None
|
||||
|
||||
|
||||
def test_discarding_a_delivered_message_is_refused(client: TestClient, db, registered, make_chat):
|
||||
"""Discard removes a row outright, so it must only ever reach one that was
|
||||
never sent."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
chat = db.get(Chat, chat_id)
|
||||
sent = chat_service.create_message(db, chat, ROLE_USER, "already gone")
|
||||
|
||||
assert client.post(f"/api/chats/{chat_id}/messages/{sent.id}/discard").status_code == 404
|
||||
assert db.get(Message, sent.id) is not None
|
||||
|
||||
|
||||
def test_send_now_delivers_and_starts_a_reply(
|
||||
client: TestClient, db, registered, make_chat, no_upstream
|
||||
):
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
chat = db.get(Chat, chat_id)
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "go on then", queued=True)
|
||||
|
||||
response = client.post(f"/api/chats/{chat_id}/messages/{waiting.id}/send-now")
|
||||
|
||||
assert response.status_code == 200
|
||||
db.refresh(waiting)
|
||||
assert waiting.queued is False
|
||||
assert len(no_upstream) == 1
|
||||
|
||||
|
||||
def test_send_now_is_refused_while_a_reply_is_running(
|
||||
client: TestClient, db, registered, make_chat, no_upstream
|
||||
):
|
||||
"""Jumping the queue is starting a second generation, which is the thing
|
||||
this whole mechanism exists to stop."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
_mid_reply(db, chat_id)
|
||||
chat = db.get(Chat, chat_id)
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "me first", queued=True)
|
||||
|
||||
response = client.post(f"/api/chats/{chat_id}/messages/{waiting.id}/send-now")
|
||||
|
||||
assert response.status_code == 409
|
||||
db.refresh(waiting)
|
||||
assert waiting.queued is True
|
||||
|
||||
|
||||
def test_neither_route_reaches_another_readers_chat(
|
||||
client: TestClient, db, registered, make_chat
|
||||
):
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
chat = db.get(Chat, chat_id)
|
||||
waiting = chat_service.create_message(db, chat, ROLE_USER, "mine", queued=True)
|
||||
|
||||
client.post("/auth/logout", follow_redirects=False)
|
||||
client.post(
|
||||
"/auth/register",
|
||||
data={"name": "Sam", "email": "sam@shire.test", "password": "potatoes-po-ta-toes"},
|
||||
follow_redirects=False,
|
||||
)
|
||||
|
||||
assert client.post(f"/api/chats/{chat_id}/messages/{waiting.id}/discard").status_code == 404
|
||||
assert client.post(f"/api/chats/{chat_id}/messages/{waiting.id}/send-now").status_code == 404
|
||||
assert db.get(Message, waiting.id) is not None
|
||||
|
||||
|
||||
# --- Everything else that touches the thread ---------------------------------
|
||||
def test_editing_is_refused_while_a_reply_is_running(
|
||||
client: TestClient, db, registered, make_chat, no_upstream
|
||||
):
|
||||
"""Editing rewinds and then starts a reply unconditionally. Pressing it
|
||||
mid-stream was a second concurrent generation behind a pencil icon, and was
|
||||
reachable before the queue existed too."""
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
_mid_reply(db, chat_id)
|
||||
first = _messages(db, chat_id)[0]
|
||||
|
||||
response = client.post(
|
||||
f"/api/chats/{chat_id}/messages/{first.id}/edit", data={"content": "rewritten"}
|
||||
)
|
||||
|
||||
assert response.status_code == 409
|
||||
db.refresh(first)
|
||||
assert first.content == "what is lembas?"
|
||||
assert len([m for m in _messages(db, chat_id) if not m.complete]) == 1
|
||||
|
||||
|
||||
def test_compaction_does_not_summarise_a_waiting_prompt(db, registered, make_chat):
|
||||
"""It would fold words no model has seen into the record, and then deliver
|
||||
them again afterwards."""
|
||||
from lembas.services import compaction as compaction_service
|
||||
|
||||
_connection(db)
|
||||
chat_id = make_chat()
|
||||
chat = db.get(Chat, chat_id)
|
||||
chat_service.create_message(db, chat, ROLE_USER, "what is lembas?")
|
||||
reply = chat_service.create_message(db, chat, ROLE_ASSISTANT, "Waybread.")
|
||||
chat_service.create_message(db, chat, ROLE_USER, "still waiting", queued=True)
|
||||
|
||||
text = compaction_service.transcript(db, chat, upto=reply)
|
||||
|
||||
assert "still waiting" not in text
|
||||
Reference in New Issue
Block a user