8000 feat(chat): edit a question you asked and get a new answer by adeelahmad · Pull Request #59 · adeelahmad/lense · GitHub
[go: up one dir, main page]

Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions docs/api.md
Original file line number Diff line number Diff line change
Expand Up @@ -920,6 +920,9 @@ question can name another (`POST /chats/{cid}/messages {content, model}`, e.g. t
a 400. Each answer records the `model` that wrote it. `POST /chats/{cid}/stop` stops the answer being written in your conversation after the piece or tool
step it's on: the stream sends `stopped`, then `done` with the saved message, whose `stopped` is true and whose
`content` is what came before (`(stopped)` when nothing had). `{stopping: false}` when nothing was being written.
`POST /chats/{cid}/messages {content, edit}` edits one of your earlier questions (`edit` is its message id): it and
everything after it are replaced by this question and its new answer; 404 when `edit` isn't a question in that
conversation. A conversation titled after the first question is retitled when that question is edited.

A conversation's `scope` narrows what it draws on: `namespaces`, `recordings`, `collections`, `speakers`, `from` and
`to`; every key narrows it further, and an empty scope is everything you can read. `collections` are ids of
Expand Down
2 changes: 2 additions & 0 deletions docs/assistant.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ then.
is writing it); what came before is saved, marked stopped. A model call already under way finishes first.
- **Reopening:** a conversation's answers keep the tools the assistant used (and with what), any notice (the model
couldn't use tools), the error when there was no answer, and their latest source check.
- **Editing a question:** Edit on a question you asked asks it again as edited; the answer and everything after it
are replaced (`POST /api/v1/chats/<id>/messages` with `edit`).
- **Changed access:** old citations are filtered by the person's current access when a conversation is reopened.
- **Choosing the model:** a conversation can use any model an admin offers (`llm.chat_models`, else whatever the model
server lists); Try another model asks a question again with a different one. Each answer records the model that
Expand Down
27 changes: 21 additions & 6 deletions fastapi_backend/app/api/v1/routes/chats.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,27 +176,42 @@ def _ev(name: str, data: Any) -> str:
async def send_message(cid: int, body: MessageCreate, user: Writer, acl: Acl, db: Db, cfg: Cfg, request: Request) -> StreamingResponse:
"""Ask a question, optionally from a page (`context`: the page, its text and any highlighted part, which the model
reads with the question). Streams events: step, approval, notice, passages, token (answer text), error, stopped (POST
/chats/{cid}/stop: what came before is saved, marked stopped), done (the saved message id)."""
/chats/{cid}/stop: what came before is saved, marked stopped), done (the saved message id).

With `edit`, one of your earlier questions is edited: it and everything after it are replaced by this question and
its new answer (404 if it isn't a question in this conversation). Without `context`, it keeps the page it was asked
from (where, and the highlighted text; not the page's text, which isn't kept)."""
c = await run_in_threadpool(_own_chat, db, cid, user)
q = body.content.strip()[:4000]
files = await run_in_threadpool(_attachments, db, body.attachments, user)
if not q and files:
q = "I attached " + ("this file." if len(files) == 1 else "these files.")
if not q:
raise HTTPException(400, "ask something")
page = body.context.model_dump() if body.context else None
# what the model reads: the question with the page it was asked from, and the files it can import
asked = chat.with_context(q, page) + chat.attached_note(files)
# this answer's model: the one asked for (400 if it isn't offered), else the conversation's while it's still offered
model = await run_in_threadpool(_model, cfg, body.model) if body.model else c.get("model")
if model and not body.model and model not in await run_in_threadpool(chat.model_choices, cfg):
model = None
title = c["title"]
page = body.context.model_dump() if body.context else None
kept = chat.shared_context(page) # what the question keeps of the page
if body.edit is not None: # only after the question checks out, so a refused edit removes nothing
try:
old = await run_in_threadpool(chat.rewind, db, cid, body.edit)
except KeyError:
raise HTTPException(404, "no such question in this conversation") from None
if title == old["content"][:80]: # titled after the question being edited: retitle it after the edit
title = "New conversation"
if page is None and old.get("context"):
page = kept = old["context"]
# what the model reads: the question with the page it was asked from, and the files it can import
asked = chat.with_context(q, page) + chat.attached_note(files)
readable = set(acl.roles)

def prepare() -> tuple[list[dict[str, Any]], list[dict[str, Any]]]:
past = chat.history(db, cid)
chat.add(db, cid, "user", q, attachments=files, context=chat.shared_context(page))
if not past and c["title"] == "New conversation":
chat.add(db, cid, "user", q, attachments=files, context=kept)
if not past and title == "New conversation":
db.q("UPDATE $r SET title = $t", r=R("chat", cid), t=q[:80])
return past, chat.retrieve(db, q, readable, c.get("scope"), cfg=cfg)

Expand Down
10 changes: 10 additions & 0 deletions fastapi_backend/app/domain/chat.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,16 @@ def add(
return mid


def rewind(db, cid, mid):
"""Remove a question of yours and everything said after it, to ask it again as edited; returns the old question
(its content and the page context it was asked with)."""
m = db.one("SELECT chat, role, content, context FROM $r", r=R("chat_message", mid))
if not m or m["chat"] != cid or m["role"] != "user":
raise KeyError(mid)
db.q("DELETE chat_message WHERE chat = $c AND record::id(id) >= $m", c=cid, m=mid)
return m


def model_choices(cfg):
"""The models people may pick: the configured one first, then llm.chat_models, or (when that's empty) whatever
the model server lists. Just the configured one when the server's list can't be had."""
Expand Down
5 changes: 5 additions & 0 deletions fastapi_backend/app/schemas/chats.py
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,11 @@ class MessageCreate(RequestModel):
model: str | None = Field(
None, description="answer this one with another model (one of GET /chats/capabilities `models`), e.g. to retry"
)
edit: int | None = Field(
None,
description="edit one of your earlier questions (its message id): it and everything after it are replaced by this "
"question and a new answer",
)


class Approval(ResponseModel):
Expand Down
64 changes: 64 additions & 0 deletions fastapi_backend/tests/api/test_chats.py
Original file line number Diff line number Diff line change
Expand Up @@ -391,6 +391,70 @@ def test_tools_stay_in_scope_and_say_what_is_missing(app, db, cfg, folder, new_c
assert box.approvals == []


def test_editing_a_question(plain, client, new_client, db, cfg, folder, llm):
seed(db, cfg, folder)
make_user(db, "ed@x.io", "editor password 1", roles={"pods": "editor", "calls": "editor"})
make_user(db, "vi@x.io", "viewer password 1", roles={"pods": "viewer"})
h = login(client, "ed@x.io", "editor password 1")
other = new_client()
hv = login(other, "vi@x.io", "viewer password 1")
cid = client.post("/api/v1/chats", headers=h, json={}).json()["id"]
for q in ("When does the shipment leave?", "Who is shipping it?", "And where to?"):
sse(client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": q}).text)
msgs = client.get(f"/api/v1/chats/{cid}", headers=h).json()["messages"]
assert len(msgs) == 6
second = msgs[2]["id"]

# an answer, someone else's or another conversation's message isn't a question you can edit here
assert client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": "x", "edit": msgs[1]["id"]}).status_code == 404
assert client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": "x", "edit": 999999}).status_code == 404
assert other.post(f"/api/v1/chats/{cid}/messages", headers=hv, json={"content": "x", "edit": second}).status_code == 404
elsewhere = client.post("/api/v1/chats", headers=h, json={}).json()["id"]
assert client.post(f"/api/v1/chats/{elsewhere}/messages", headers=h, json={"content": "x", "edit": second}).status_code == 404
assert len(client.get(f"/api/v1/chats/{cid}", headers=h).json()["messages"]) == 6 # nothing was removed

# editing the second question replaces it and everything after it; the model sees only what came before
llm.seen.clear()
ev = sse(client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": "Who ships it, exactly?", "edit": second}).text)
assert ev["done"]
sent = [m["content"] for m in llm.seen[-1]["messages"] if m["role"] == "user"]
assert "When does the shipment leave?" in sent[0] and not any("Who is shipping it?" in s or "And where to?" in s for s in sent)
c = client.get(f"/api/v1/chats/{cid}", headers=h).json()
assert [m["content"] for m in c["messages"] if m["role"] == "user"] == ["When does the shipment leave?", "Who ships it, exactly?"]
assert [m["role"] for m in c["messages"]] == ["user", "assistant", "user", "assistant"]
assert c["title"] == "When does the shipment leave?"

# editing the first question retitles a conversation titled after it, but keeps a title you set
first = c["messages"][0]["id"]
sse(client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": "When is the Friday shipment?", "edit": first}).text)
c = client.get(f"/api/v1/chats/{cid}", headers=h).json()
assert ([m["content"] for m in c["messages"] if m["role"] == "user"], c["title"]) == (
["When is the Friday shipment?"],
"When is the Friday shipment?",
)
assert client.patch(f"/api/v1/chats/{cid}", headers=h, json={"title": "Shipping"}).status_code == 200
first = c["messages"][0]["id"]
sse(client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": "When does it ship?", "edit": first}).text)
assert client.get(f"/api/v1/chats/{cid}", headers=h).json()["title"] == "Shipping"

# a refused edit (a model that isn't offered) removes nothing
n = len(client.get(f"/api/v1/chats/{cid}", headers=h).json()["messages"])
assert (
client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": "x", "edit": first, "model": "gpt-9"}).status_code == 400
)
assert len(client.get(f"/api/v1/chats/{cid}", headers=h).json()["messages"]) == n

# a question asked from a page keeps the page and the highlighted text when edited
page = {"url": "/library", "title": "Library", "text": "Shipment report", "selection": "Dyno shipment"}
sse(client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": "When?", "context": page}).text)
asked = client.get(f"/api/v1/chats/{cid}", headers=h).json()["messages"][-2]
llm.seen.clear()
sse(client.post(f"/api/v1/chats/{cid}/messages", headers=h, json={"content": "When exactly?", "edit": asked["id"]}).text)
assert '"""\nDyno shipment\n"""' in llm.seen[-1]["messages"][-1]["content"]
edited = client.get(f"/api/v1/chats/{cid}", headers=h).json()["messages"][-2]
assert (edited["content"], edited["context"]) == ("When exactly?", asked["context"])


def test_asking_from_a_page(plain, client, db, cfg, folder, llm):
"""Chat on any page: the model reads the page and the highlighted part with the question; the question keeps where
it was asked and what was highlighted, not the page's text."""
Expand Down
60 changes: 59 additions & 1 deletion nextjs-frontend/__tests__/chat.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render, screen } from "@testing-library/react";
import { fireEvent, render, screen } from "@testing-library/react";
import "@testing-library/jest-dom";

import {
Expand All @@ -14,6 +14,18 @@ import {
} from "@/components/chat/cite";
import { parseBlocks, RichText } from "@/components/chat/rich-text";
import { applyEvent, newTurn, savedSteps, stepCall } from "@/components/chat/stream";
import { UserBubble } from "@/components/chat/user-bubble";

jest.mock("@/components/chat/attachments", () => ({
SentFiles: ({ files }: { files: { filename: string }[] }) => (
<ul aria-label="Files sent">
{files.map((f) => (
<li key={f.filename}>{f.filename}</li>
))}
</ul>
),
}));
import { TooltipProvider } from "@/components/ui/tooltip";

describe("citations", () => {
it("splits [n], [n, m] and [n][m]", () => {
Expand Down Expand Up @@ -177,3 +189,49 @@ describe("rich text", () => {
expect(screen.getByText(/<b>not html<\/b>/)).toBeInTheDocument();
});
});

describe("editing a question", () => {
it("edits in place and sends the new text, or cancels", () => {
const >
render(
<TooltipProvider>
<UserBubble text="When does it ship?" />
</TooltipProvider>,
);
fireEvent.click(screen.getByRole("button", { name: "Edit" }));
const box = screen.getByRole("textbox", { name: "Edit your question" });
expect(box).toHaveValue("When does it ship?");
fireEvent.change(box, { target: { value: " When does the Friday shipment leave? " } });
fireEvent.keyDown(box, { key: "Enter" });
expect(onEdit).toHaveBeenCalledWith("When does the Friday shipment leave?");
expect(screen.queryByRole("textbox")).not.toBeInTheDocument();

fireEvent.click(screen.getByRole("button", { name: "Edit" }));
fireEvent.change(screen.getByRole("textbox"), { target: { value: "something else" } });
fireEvent.click(screen.getByRole("button", { name: "Cancel" }));
expect(onEdit).toHaveBeenCalledTimes(1);
expect(screen.getByText("When does it ship?")).toBeInTheDocument();
});

it("re-sends a files-only question's files, with the text you add", () => {
const >
render(
<TooltipProvider>
<UserBubble text="I attached this file." files={[{ filename: "a.pdf", size: 3 }]} />
</TooltipProvider>,
);
expect(screen.queryByText("I attached this file.")).not.toBeInTheDocument();
expect(screen.getByText("a.pdf")).toBeInTheDocument();
fireEvent.click(screen.getByRole("button", { name: "Edit" }));
const box = screen.getByRole("textbox", { name: "Edit your question" });
expect(box).toHaveValue("");
fireEvent.change(box, { target: { value: "Summarise it" } });
fireEvent.click(screen.getByRole("button", { name: "Send" }));
expect(onEdit).toHaveBeenCalledWith("Summarise it");
});

it("can't be edited without a handler, e.g. while an answer is written", () => {
render(<UserBubble text="Hi" />);
expect(screen.queryByRole("button", { name: "Edit" })).not.toBeInTheDocument();
});
});
4 changes: 4 additions & 0 deletions nextjs-frontend/app/openapi-client/sdk.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5563,6 +5563,10 @@ export class Chats {
* Ask a question, optionally from a page (`context`: the page, its text and any highlighted part, which the model
* reads with the question). Streams events: step, approval, notice, passages, token (answer text), error, stopped (POST
* /chats/{cid}/stop: what came before is saved, marked stopped), done (the saved message id).
*
* With `edit`, one of your earlier questions is edited: it and everything after it are replaced by this question and
* its new answer (404 if it isn't a question in this conversation). Without `context`, it keeps the page it was asked
* from (where, and the highlighted text; not the page's text, which isn't kept).
*/
public static sendMessage<ThrowOnError extends boolean = false>(
options: Options<SendMessageData, ThrowOnError, unknown>,
Expand Down
6 changes: 6 additions & 0 deletions nextjs-frontend/app/openapi-client/types.gen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5357,6 +5357,12 @@ export type MessageCreate = {
* answer this one with another model (one of GET /chats/capabilities `models`), e.g. to retry
*/
model?: string | null;
/**
* Edit
*
* edit one of your earlier questions (its message id): it and everything after it are replaced by this question and a new answer
*/
edit?: number | null;
};

/**
Expand Down
Loading
Loading
0