Refusing can say why, and the why is an instruction
"Don't" told the model it was refused and nothing else, so it did the one sensible thing left and asked what you would rather -- a whole round spent on something you knew when you pressed the button. "Give reason" opens a box beside it, and what you write goes back with the refusal. The reason changes what the model is *told*, not only what it reads, and that is the whole of the feature. `_not_allowed` branches: given nothing to go on, "say what you were going to do and ask what they would prefer" is right; given a reason it is exactly wrong, because the answer is already on the screen above and the model spends a round asking for it again. So it is pointed at the reason and told to carry on from it. The "do not look for a way round" half is kept either way -- that half is about the refusal and holds regardless. A card-level field rather than `text.<key>`. One card covers everything in the round for the reason the primitive exists, so one reason answers the round; and on an approval card `text.<key>` already means a corrected command, which is a different thing arriving in the same shape. Read only on a refusal, so a reason typed and then abandoned by pressing Allow cannot travel with a permission. Bounded where the Reply is built, so nothing downstream thinks about length, and put on the tool event as well as in the result -- a transcript saying a step was refused without saying why is one you had to have been watching to understand. It is also the one thing in a tool result that is genuinely not untrusted: the reader's own words, stated as theirs, needing no fence. Both halves of the control are in the DOM with one hidden and the textarea disabled while hidden, which is the rule the edit box beside it already states: a field created by a click submits nothing when the click handler fails, and an empty `reason` arriving would have to be told from one somebody cleared. The version bump is not incidental. chat.css changed and the service worker caches it under a name keyed on the version, so without it the first reload serves the old stylesheet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,3 +1,3 @@
|
|||||||
"""LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints."""
|
"""LLeMbas - a Middle-earth themed web UI for OpenAI-compatible LLM endpoints."""
|
||||||
|
|
||||||
__version__ = "0.6.4"
|
__version__ = "0.6.5"
|
||||||
|
|||||||
@@ -1584,6 +1584,14 @@ async def answer_interaction(
|
|||||||
form = await request.form()
|
form = await request.form()
|
||||||
verdict = str(form.get("verdict") or "").strip()
|
verdict = str(form.get("verdict") or "").strip()
|
||||||
|
|
||||||
|
# Why they refused, in their own words. A card-level field rather than a
|
||||||
|
# `text.<key>` one: the card covers everything in the round, so one reason
|
||||||
|
# answers the round -- and on an approval card `text.<key>` already means a
|
||||||
|
# corrected command, which is a different thing arriving in the same shape.
|
||||||
|
# Only read on a refusal, so a reason typed and then abandoned by pressing
|
||||||
|
# Allow cannot travel with a permission.
|
||||||
|
reason = str(form.get("reason") or "").strip() if verdict == interaction.DENY else ""
|
||||||
|
|
||||||
# Gathered in two passes because one field can now arrive several times: a
|
# Gathered in two passes because one field can now arrive several times: a
|
||||||
# question the model marked `multiple` is checkboxes, and every ticked one
|
# question the model marked `multiple` is checkboxes, and every ticked one
|
||||||
# posts under the same name. A `setdefault` would keep the first and lose
|
# posts under the same name. A `setdefault` would keep the first and lose
|
||||||
@@ -1642,6 +1650,7 @@ async def answer_interaction(
|
|||||||
interaction_id,
|
interaction_id,
|
||||||
verdict=verdict,
|
verdict=verdict,
|
||||||
answers=answers,
|
answers=answers,
|
||||||
|
reason=reason,
|
||||||
)
|
)
|
||||||
|
|
||||||
response = Response(status_code=status.HTTP_204_NO_CONTENT)
|
response = Response(status_code=status.HTTP_204_NO_CONTENT)
|
||||||
|
|||||||
@@ -288,6 +288,7 @@ def answer(
|
|||||||
*,
|
*,
|
||||||
verdict: str = "",
|
verdict: str = "",
|
||||||
answers: dict[str, str] | None = None,
|
answers: dict[str, str] | None = None,
|
||||||
|
reason: str = "",
|
||||||
) -> bool:
|
) -> bool:
|
||||||
"""Resolve whichever running reply is parked on this interruption.
|
"""Resolve whichever running reply is parked on this interruption.
|
||||||
|
|
||||||
@@ -300,7 +301,7 @@ def answer(
|
|||||||
if generation.chat_id != chat_id or pending is None or pending.id != interaction_id:
|
if generation.chat_id != chat_id or pending is None or pending.id != interaction_id:
|
||||||
continue
|
continue
|
||||||
outcome = verdict if verdict in _VERDICTS else interaction.ANSWER
|
outcome = verdict if verdict in _VERDICTS else interaction.ANSWER
|
||||||
return pending.resolve(outcome, answers=answers)
|
return pending.resolve(outcome, answers=answers, reason=reason)
|
||||||
return False
|
return False
|
||||||
|
|
||||||
|
|
||||||
@@ -1629,6 +1630,20 @@ def _not_allowed(item: interaction.Item, reply: interaction.Reply) -> ToolOutcom
|
|||||||
Told plainly, and told to stop rather than to try again: a model that reads
|
Told plainly, and told to stop rather than to try again: a model that reads
|
||||||
"not allowed" as "not allowed *that way*" will spend the rest of the reply
|
"not allowed" as "not allowed *that way*" will spend the rest of the reply
|
||||||
looking for a way round, which is the opposite of what the refusal meant.
|
looking for a way round, which is the opposite of what the refusal meant.
|
||||||
|
|
||||||
|
A refusal that came with a reason is told differently, and the difference is
|
||||||
|
the whole point of offering the box. "Ask what they would prefer" is the
|
||||||
|
right thing to say to a model that has been given nothing to go on, and
|
||||||
|
exactly the wrong thing to say to one that has just been told -- it spends a
|
||||||
|
round asking a question whose answer is on the screen above it. So when there
|
||||||
|
is a reason the model is pointed at it and told to carry on from it, and the
|
||||||
|
"do not look for a way round" half is kept, because that half is about the
|
||||||
|
refusal and holds either way.
|
||||||
|
|
||||||
|
The reason is the *reader's* words, not a model's and not a machine's, which
|
||||||
|
is why it is stated as theirs and needs no fence: this is the one thing in a
|
||||||
|
tool result that is not untrusted. It is bounded at `MAX_REASON_CHARS` when
|
||||||
|
the reply is built, so nothing here has to think about length.
|
||||||
"""
|
"""
|
||||||
event = {
|
event = {
|
||||||
"name": item.tool_name,
|
"name": item.tool_name,
|
||||||
@@ -1643,6 +1658,17 @@ def _not_allowed(item: interaction.Item, reply: interaction.Reply) -> ToolOutcom
|
|||||||
"about to do and why.",
|
"about to do and why.",
|
||||||
{**event, "status": "error", "error": "Not answered."},
|
{**event, "status": "error", "error": "Not answered."},
|
||||||
)
|
)
|
||||||
|
if reply.reason:
|
||||||
|
return ToolOutcome(
|
||||||
|
f"They declined this, and said why:\n\n{reply.reason}\n\n"
|
||||||
|
"Take that as their instruction and carry on from it. Do not try the "
|
||||||
|
"same thing another way, and do not ask them to repeat what they have "
|
||||||
|
"just told you.",
|
||||||
|
# On the event as well as in the result, so somebody scrolling back
|
||||||
|
# through the transcript can see why a step was refused rather than
|
||||||
|
# only that it was.
|
||||||
|
{**event, "status": "error", "error": f"Declined: {reply.reason}"},
|
||||||
|
)
|
||||||
return ToolOutcome(
|
return ToolOutcome(
|
||||||
"They declined this. Do not try it another way — say what you were "
|
"They declined this. Do not try it another way — say what you were "
|
||||||
"going to do and ask what they would prefer.",
|
"going to do and ask what they would prefer.",
|
||||||
|
|||||||
@@ -66,6 +66,12 @@ OTHER = "__other__"
|
|||||||
# card somebody is meant to read at a glance.
|
# card somebody is meant to read at a glance.
|
||||||
MAX_OPTION_CHARS = 240
|
MAX_OPTION_CHARS = 240
|
||||||
|
|
||||||
|
# How much of a refusal's reason is carried back to the model. Generous, because
|
||||||
|
# this is the reader saying what they want instead and truncating that mid-clause
|
||||||
|
# is worse than the tokens it saves -- but bounded, because it lands in a tool
|
||||||
|
# result inside a request that already has a window to fit in.
|
||||||
|
MAX_REASON_CHARS = 2000
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
@dataclass(frozen=True)
|
||||||
class Option:
|
class Option:
|
||||||
@@ -143,7 +149,9 @@ class Interruption:
|
|||||||
def kind(self) -> str:
|
def kind(self) -> str:
|
||||||
return KIND_QUESTION if any(i.kind == KIND_QUESTION for i in self.items) else KIND_APPROVAL
|
return KIND_QUESTION if any(i.kind == KIND_QUESTION for i in self.items) else KIND_APPROVAL
|
||||||
|
|
||||||
def resolve(self, outcome: str, *, answers: dict[str, str] | None = None) -> bool:
|
def resolve(
|
||||||
|
self, outcome: str, *, answers: dict[str, str] | None = None, reason: str = ""
|
||||||
|
) -> bool:
|
||||||
"""Complete this pause. Idempotent -- a second answer is ignored.
|
"""Complete this pause. Idempotent -- a second answer is ignored.
|
||||||
|
|
||||||
Returns whether this call was the one that answered it, which is what
|
Returns whether this call was the one that answered it, which is what
|
||||||
@@ -152,7 +160,13 @@ class Interruption:
|
|||||||
"""
|
"""
|
||||||
if self._future is None or self._future.done():
|
if self._future is None or self._future.done():
|
||||||
return False
|
return False
|
||||||
self._future.set_result(Reply(outcome=outcome, answers=dict(answers or {})))
|
self._future.set_result(
|
||||||
|
Reply(
|
||||||
|
outcome=outcome,
|
||||||
|
answers=dict(answers or {}),
|
||||||
|
reason=reason.strip()[:MAX_REASON_CHARS],
|
||||||
|
)
|
||||||
|
)
|
||||||
return True
|
return True
|
||||||
|
|
||||||
|
|
||||||
@@ -162,11 +176,19 @@ class Reply:
|
|||||||
|
|
||||||
`answers` is keyed by `Item.key`, so a card carrying four questions comes
|
`answers` is keyed by `Item.key`, so a card carrying four questions comes
|
||||||
back as four answers in one go. An approval carries none: the verdict is
|
back as four answers in one go. An approval carries none: the verdict is
|
||||||
the whole of it.
|
the whole of it -- except for `reason`.
|
||||||
|
|
||||||
|
`reason` is why the reader refused, in their own words, and it belongs to
|
||||||
|
the *card* rather than to an item. The card already covers everything in the
|
||||||
|
round for the reason `interaction` opens with, one verdict answers the lot,
|
||||||
|
and somebody who says "not in that directory" is saying it about the round.
|
||||||
|
Keeping it off `answers` also keeps it clear of `text.<key>`, which on an
|
||||||
|
approval card already means something else entirely -- a corrected command.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
outcome: str
|
outcome: str
|
||||||
answers: dict[str, str] = field(default_factory=dict)
|
answers: dict[str, str] = field(default_factory=dict)
|
||||||
|
reason: str = ""
|
||||||
|
|
||||||
@property
|
@property
|
||||||
def permitted(self) -> bool:
|
def permitted(self) -> bool:
|
||||||
@@ -239,6 +261,7 @@ __all__ = [
|
|||||||
"KIND_QUESTION",
|
"KIND_QUESTION",
|
||||||
"MAX_OPTIONS",
|
"MAX_OPTIONS",
|
||||||
"MAX_QUESTIONS",
|
"MAX_QUESTIONS",
|
||||||
|
"MAX_REASON_CHARS",
|
||||||
"PERMITTED",
|
"PERMITTED",
|
||||||
"Interruption",
|
"Interruption",
|
||||||
"Item",
|
"Item",
|
||||||
|
|||||||
@@ -649,6 +649,24 @@
|
|||||||
.interaction__write { display: flex; gap: var(--sp-2); flex: 1 1 16rem; min-width: 0; }
|
.interaction__write { display: flex; gap: var(--sp-2); flex: 1 1 16rem; min-width: 0; }
|
||||||
.interaction__write .input { flex: 1; min-width: 0; }
|
.interaction__write .input { flex: 1; min-width: 0; }
|
||||||
|
|
||||||
|
/* Saying why, instead of only saying no. `flex: 1 0 100%` takes the whole row
|
||||||
|
of the wrapping action bar rather than squeezing in beside Allow: this is a
|
||||||
|
sentence being written, not a fourth button. */
|
||||||
|
.interaction__deny {
|
||||||
|
flex: 1 0 100%;
|
||||||
|
display: flex;
|
||||||
|
flex-direction: column;
|
||||||
|
gap: var(--sp-2);
|
||||||
|
min-width: 0;
|
||||||
|
}
|
||||||
|
.interaction__deny-input { width: 100%; resize: vertical; }
|
||||||
|
.interaction__deny-actions {
|
||||||
|
display: flex;
|
||||||
|
flex-wrap: wrap;
|
||||||
|
gap: var(--sp-2);
|
||||||
|
align-items: center;
|
||||||
|
}
|
||||||
|
|
||||||
/* --- Stop, notes and editing ---------------------------------------------- */
|
/* --- Stop, notes and editing ---------------------------------------------- */
|
||||||
.msg__status { font-size: var(--text-xs); color: var(--ink-faint); font-style: italic; }
|
.msg__status { font-size: var(--text-xs); color: var(--ink-faint); font-style: italic; }
|
||||||
.msg__status:empty { display: none; }
|
.msg__status:empty { display: none; }
|
||||||
|
|||||||
@@ -152,16 +152,59 @@
|
|||||||
</div>
|
</div>
|
||||||
{% endfor %}
|
{% endfor %}
|
||||||
|
|
||||||
<div class="interaction__actions">
|
{#
|
||||||
|
Refusing, with or without saying why.
|
||||||
|
|
||||||
|
A bare "Don't" tells the model it was refused and nothing else, so it
|
||||||
|
does the only sensible thing left and asks what you would rather -- which
|
||||||
|
costs a round to say something you already knew when you pressed the
|
||||||
|
button. The box is how you skip that round: "not in that directory, use
|
||||||
|
/srv" is a refusal and an instruction in one, and `_not_allowed` changes
|
||||||
|
what it tells the model when there is one.
|
||||||
|
|
||||||
|
Both halves are always in the DOM with one hidden, for the reason the
|
||||||
|
edit box above gives: a field created by a click is a field that submits
|
||||||
|
nothing when the click handler fails. And the textarea is disabled while
|
||||||
|
hidden, so a card refused plainly cannot post an empty `reason` that
|
||||||
|
would have to be told apart from one somebody cleared.
|
||||||
|
#}
|
||||||
|
<div class="interaction__actions" x-data="{ explaining: false }">
|
||||||
<button class="btn btn--primary" type="submit" name="verdict" value="allow">
|
<button class="btn btn--primary" type="submit" name="verdict" value="allow">
|
||||||
{{ icon("check", "icon--sm") }} Allow
|
{{ icon("check", "icon--sm") }} Allow
|
||||||
</button>
|
</button>
|
||||||
<button class="btn" type="submit" name="verdict" value="allow_always">
|
<button class="btn" type="submit" name="verdict" value="allow_always">
|
||||||
Always allow this
|
Always allow this
|
||||||
</button>
|
</button>
|
||||||
<button class="btn btn--danger" type="submit" name="verdict" value="deny">
|
<button class="btn btn--danger" type="submit" name="verdict" value="deny"
|
||||||
|
x-show="!explaining">
|
||||||
{{ icon("x", "icon--sm") }} Don't
|
{{ icon("x", "icon--sm") }} Don't
|
||||||
</button>
|
</button>
|
||||||
|
<button class="btn" type="button" x-show="!explaining"
|
||||||
|
@click="explaining = true; $nextTick(() => $refs.reason.focus())">
|
||||||
|
{{ icon("pencil", "icon--sm") }} Give reason
|
||||||
|
</button>
|
||||||
|
|
||||||
|
<div class="interaction__deny" x-show="explaining" x-cloak>
|
||||||
|
<label class="visually-hidden" for="deny-reason">
|
||||||
|
Why not, and what to do instead
|
||||||
|
</label>
|
||||||
|
{# Never type="password", for the reason the "Something else" box
|
||||||
|
above states: a chat transcript is not a place to keep secrets. #}
|
||||||
|
{# `--deny-input` and not `--reason`: `.interaction__reason` already
|
||||||
|
means *our* reason for stopping, which is the opposite direction. #}
|
||||||
|
<textarea class="textarea interaction__deny-input" id="deny-reason"
|
||||||
|
name="reason" rows="2" spellcheck="false" x-ref="reason"
|
||||||
|
:disabled="!explaining"
|
||||||
|
placeholder="Why not, and what to do instead…"></textarea>
|
||||||
|
<div class="interaction__deny-actions">
|
||||||
|
<button class="btn btn--danger" type="submit" name="verdict" value="deny">
|
||||||
|
{{ icon("x", "icon--sm") }} Don't, and tell it why
|
||||||
|
</button>
|
||||||
|
<button class="btn btn--sm" type="button" @click="explaining = false">
|
||||||
|
Cancel
|
||||||
|
</button>
|
||||||
|
</div>
|
||||||
|
</div>
|
||||||
</div>
|
</div>
|
||||||
{% endif %}
|
{% endif %}
|
||||||
</form>
|
</form>
|
||||||
|
|||||||
@@ -105,7 +105,7 @@ def _shell_call(command: str, *, call_id: str = "c1") -> dict:
|
|||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
async def _authorise_with(context, calls, *, answers, verdict=interaction.ALLOW):
|
async def _authorise_with(context, calls, *, answers, verdict=interaction.ALLOW, reason=""):
|
||||||
"""Run `_authorise` and answer the card it puts up."""
|
"""Run `_authorise` and answer the card it puts up."""
|
||||||
generation = generation_service.Generation(chat_id="x", message_id="y")
|
generation = generation_service.Generation(chat_id="x", message_id="y")
|
||||||
arguments = generation_service._arguments_for(context, calls)
|
arguments = generation_service._arguments_for(context, calls)
|
||||||
@@ -120,7 +120,7 @@ async def _authorise_with(context, calls, *, answers, verdict=interaction.ALLOW)
|
|||||||
await asyncio.sleep(0.01)
|
await asyncio.sleep(0.01)
|
||||||
|
|
||||||
pending = generation.pending
|
pending = generation.pending
|
||||||
pending.resolve(verdict, answers=answers)
|
pending.resolve(verdict, answers=answers, reason=reason)
|
||||||
decided, allowed, edited = await task
|
decided, allowed, edited = await task
|
||||||
return arguments, decided, allowed, edited, pending
|
return arguments, decided, allowed, edited, pending
|
||||||
|
|
||||||
@@ -223,6 +223,47 @@ async def test_declining_ignores_the_edit(db, user_id, machine):
|
|||||||
assert arguments[0]["command"] == "pytest"
|
assert arguments[0]["command"] == "pytest"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_refusal_with_a_reason_answers_every_call_in_the_round(
|
||||||
|
db, user_id, machine
|
||||||
|
):
|
||||||
|
"""One card covers the round, so one reason answers the round.
|
||||||
|
|
||||||
|
That is the design claim worth pinning: the reader said "not on production"
|
||||||
|
about what they were shown, and what they were shown was both commands. A
|
||||||
|
reason attached to one of them would be the card describing something other
|
||||||
|
than what it asked about.
|
||||||
|
"""
|
||||||
|
context = _context(db, user_id, machine)
|
||||||
|
calls = [_shell_call("pytest", call_id="c1"), _shell_call("make deploy", call_id="c2")]
|
||||||
|
|
||||||
|
_arguments, decided, allowed, _edited, _ = await _authorise_with(
|
||||||
|
context,
|
||||||
|
calls,
|
||||||
|
answers={},
|
||||||
|
verdict=interaction.DENY,
|
||||||
|
reason="not on production",
|
||||||
|
)
|
||||||
|
|
||||||
|
assert allowed == set(), "nothing ran"
|
||||||
|
assert set(decided) == {0, 1}, "and every call was answered without the runner"
|
||||||
|
for outcome in decided.values():
|
||||||
|
assert "not on production" in outcome.content
|
||||||
|
assert outcome.event["error"] == "Declined: not on production"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_refusal_with_no_reason_is_unchanged(db, user_id, machine):
|
||||||
|
"""The path everything already took, still taking it."""
|
||||||
|
context = _context(db, user_id, machine)
|
||||||
|
calls = [_shell_call("pytest")]
|
||||||
|
|
||||||
|
_arguments, decided, _allowed, _edited, _ = await _authorise_with(
|
||||||
|
context, calls, answers={}, verdict=interaction.DENY
|
||||||
|
)
|
||||||
|
|
||||||
|
assert decided[0].event["error"] == "Declined."
|
||||||
|
assert "ask what they would prefer" in decided[0].content
|
||||||
|
|
||||||
|
|
||||||
async def test_only_the_edited_call_in_a_round_is_changed(db, user_id, machine):
|
async def test_only_the_edited_call_in_a_round_is_changed(db, user_id, machine):
|
||||||
"""One card covers the whole round, and the answers are keyed per item."""
|
"""One card covers the whole round, and the answers are keyed per item."""
|
||||||
context = _context(db, user_id, machine)
|
context = _context(db, user_id, machine)
|
||||||
|
|||||||
@@ -766,7 +766,7 @@ def test_the_endpoint_gathers_every_answer_at_once(client, db, registered, user_
|
|||||||
chat_id, message_id = _chat_that_can_ask(db, user_id)
|
chat_id, message_id = _chat_that_can_ask(db, user_id)
|
||||||
seen: dict = {}
|
seen: dict = {}
|
||||||
|
|
||||||
def capture(chat, interaction_id, *, verdict="", answers=None):
|
def capture(chat, interaction_id, *, verdict="", answers=None, reason=""):
|
||||||
seen["chat"] = chat
|
seen["chat"] = chat
|
||||||
seen["id"] = interaction_id
|
seen["id"] = interaction_id
|
||||||
seen["verdict"] = verdict
|
seen["verdict"] = verdict
|
||||||
@@ -811,7 +811,7 @@ def test_the_endpoint_passes_a_verdict_through_untouched(client, db, registered,
|
|||||||
from lembas.api import chats as chats_api
|
from lembas.api import chats as chats_api
|
||||||
|
|
||||||
original = chats_api.generation_service.answer
|
original = chats_api.generation_service.answer
|
||||||
chats_api.generation_service.answer = lambda c, i, *, verdict="", answers=None: (
|
chats_api.generation_service.answer = lambda c, i, *, verdict="", answers=None, reason="": (
|
||||||
seen.update(verdict=verdict, answers=answers) or True
|
seen.update(verdict=verdict, answers=answers) or True
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
@@ -921,8 +921,9 @@ def test_several_ticked_answers_all_come_back(client, db, registered, user_id):
|
|||||||
from lembas.api import chats as chats_api
|
from lembas.api import chats as chats_api
|
||||||
|
|
||||||
original = chats_api.generation_service.answer
|
original = chats_api.generation_service.answer
|
||||||
chats_api.generation_service.answer = lambda chat, iid, *, verdict="", answers=None: (
|
chats_api.generation_service.answer = (
|
||||||
seen.update(answers=answers) or True
|
lambda chat, iid, *, verdict="", answers=None, reason="": seen.update(answers=answers)
|
||||||
|
or True
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
client.post(
|
client.post(
|
||||||
@@ -944,8 +945,9 @@ def test_something_else_ticked_with_an_empty_box_says_nothing(client, db, regist
|
|||||||
from lembas.api import chats as chats_api
|
from lembas.api import chats as chats_api
|
||||||
|
|
||||||
original = chats_api.generation_service.answer
|
original = chats_api.generation_service.answer
|
||||||
chats_api.generation_service.answer = lambda chat, iid, *, verdict="", answers=None: (
|
chats_api.generation_service.answer = (
|
||||||
seen.update(answers=answers) or True
|
lambda chat, iid, *, verdict="", answers=None, reason="": seen.update(answers=answers)
|
||||||
|
or True
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
client.post(
|
client.post(
|
||||||
@@ -965,8 +967,9 @@ def test_a_ticked_option_and_something_else_come_back_together(client, db, regis
|
|||||||
from lembas.api import chats as chats_api
|
from lembas.api import chats as chats_api
|
||||||
|
|
||||||
original = chats_api.generation_service.answer
|
original = chats_api.generation_service.answer
|
||||||
chats_api.generation_service.answer = lambda chat, iid, *, verdict="", answers=None: (
|
chats_api.generation_service.answer = (
|
||||||
seen.update(answers=answers) or True
|
lambda chat, iid, *, verdict="", answers=None, reason="": seen.update(answers=answers)
|
||||||
|
or True
|
||||||
)
|
)
|
||||||
try:
|
try:
|
||||||
client.post(
|
client.post(
|
||||||
@@ -977,3 +980,168 @@ def test_a_ticked_option_and_something_else_come_back_together(client, db, regis
|
|||||||
chats_api.generation_service.answer = original
|
chats_api.generation_service.answer = original
|
||||||
|
|
||||||
assert seen["answers"] == {"q0": "Tea, or juice"}
|
assert seen["answers"] == {"q0": "Tea, or juice"}
|
||||||
|
|
||||||
|
|
||||||
|
# --- Refusing, and saying why --------------------------------------------------
|
||||||
|
# A bare "Don't" tells the model it was refused and nothing else, so it asks what
|
||||||
|
# you would rather -- a whole round spent on something you knew when you pressed
|
||||||
|
# the button. The box is how that round is skipped, and these are the two halves
|
||||||
|
# that have to hold: the reason reaches the model, and it changes what the model
|
||||||
|
# is told to do next.
|
||||||
|
def _approval(key: str = "a0", **kwargs) -> interaction.Item:
|
||||||
|
return interaction.Item(
|
||||||
|
index=0,
|
||||||
|
key=key,
|
||||||
|
kind=interaction.KIND_APPROVAL,
|
||||||
|
tool_name="shell_run",
|
||||||
|
title="Run a command",
|
||||||
|
detail="rm -rf build",
|
||||||
|
**kwargs,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_refusal_can_carry_a_reason():
|
||||||
|
pause = interaction.build("p", [_approval()], timeout=5)
|
||||||
|
pause.resolve(interaction.DENY, reason=" not in that directory, use /srv ")
|
||||||
|
|
||||||
|
reply = await pause._future
|
||||||
|
|
||||||
|
assert reply.permitted is False
|
||||||
|
assert reply.reason == "not in that directory, use /srv", "trimmed, as typed"
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_plain_refusal_carries_none():
|
||||||
|
"""Which is what keeps the two wordings apart downstream."""
|
||||||
|
pause = interaction.build("p", [_approval()], timeout=5)
|
||||||
|
pause.resolve(interaction.DENY)
|
||||||
|
|
||||||
|
assert (await pause._future).reason == ""
|
||||||
|
|
||||||
|
|
||||||
|
async def test_a_reason_is_bounded():
|
||||||
|
"""It lands in a tool result inside a request that has a window to fit in."""
|
||||||
|
pause = interaction.build("p", [_approval()], timeout=5)
|
||||||
|
pause.resolve(interaction.DENY, reason="x" * (interaction.MAX_REASON_CHARS + 500))
|
||||||
|
|
||||||
|
assert len((await pause._future).reason) == interaction.MAX_REASON_CHARS
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reason_reaches_the_model_and_redirects_it():
|
||||||
|
"""The instruction has to change, not just the text. "Ask what they would
|
||||||
|
prefer" is right for a model given nothing to go on and exactly wrong for one
|
||||||
|
that has just been told -- it spends a round asking a question whose answer is
|
||||||
|
on the screen above it."""
|
||||||
|
outcome = generation_service._not_allowed(
|
||||||
|
_approval(), interaction.Reply(outcome=interaction.DENY, reason="use /srv instead")
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "use /srv instead" in outcome.content
|
||||||
|
assert "carry on from it" in outcome.content
|
||||||
|
assert "do not ask them to repeat" in outcome.content.lower()
|
||||||
|
assert "ask what they would prefer" not in outcome.content
|
||||||
|
# The half that holds either way: a refusal is not an invitation to find
|
||||||
|
# another route to the same thing.
|
||||||
|
assert "Do not try the same thing another way" in outcome.content
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_refusal_without_a_reason_is_worded_as_it_always_was():
|
||||||
|
outcome = generation_service._not_allowed(
|
||||||
|
_approval(), interaction.Reply(outcome=interaction.DENY)
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "ask what they would prefer" in outcome.content
|
||||||
|
assert outcome.event["error"] == "Declined."
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_reason_is_in_the_transcript_too():
|
||||||
|
"""So somebody scrolling back sees why a step was refused rather than only
|
||||||
|
that it was."""
|
||||||
|
outcome = generation_service._not_allowed(
|
||||||
|
_approval(), interaction.Reply(outcome=interaction.DENY, reason="wrong host")
|
||||||
|
)
|
||||||
|
|
||||||
|
assert outcome.event["error"] == "Declined: wrong host"
|
||||||
|
|
||||||
|
|
||||||
|
def test_an_unanswered_card_is_not_a_refusal_with_a_reason():
|
||||||
|
"""Nobody said anything, so there is nothing to pass on -- and the wording
|
||||||
|
has to stay the one about nobody answering."""
|
||||||
|
outcome = generation_service._not_allowed(
|
||||||
|
_approval(), interaction.Reply(outcome=interaction.EXPIRED)
|
||||||
|
)
|
||||||
|
|
||||||
|
assert "Nobody answered" in outcome.content
|
||||||
|
assert outcome.event["error"] == "Not answered."
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_card_offers_the_box_and_names_the_field(client, db, registered, user_id):
|
||||||
|
pause = interaction.Interruption(id="p1", items=(_approval(),))
|
||||||
|
|
||||||
|
html = _render(pause)
|
||||||
|
|
||||||
|
assert "Give reason" in html
|
||||||
|
assert 'name="reason"' in html
|
||||||
|
# Both halves in the DOM with one hidden, for the reason the edit box gives:
|
||||||
|
# a field created by a click submits nothing when the click handler fails.
|
||||||
|
assert 'x-show="explaining"' in html
|
||||||
|
# Disabled while hidden, so a card refused plainly cannot post an empty
|
||||||
|
# `reason` that would have to be told apart from one somebody cleared.
|
||||||
|
assert ':disabled="!explaining"' in html
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_question_card_offers_no_deny_box():
|
||||||
|
"""It is not a refusal, and it has its own free-text row already."""
|
||||||
|
html = _render(interaction.Interruption(id="p1", items=(_item(),)))
|
||||||
|
|
||||||
|
assert "Give reason" not in html
|
||||||
|
assert 'name="reason"' not in html
|
||||||
|
|
||||||
|
|
||||||
|
def test_the_endpoint_passes_the_reason_on(client, db, registered, user_id):
|
||||||
|
chat_id, _message_id = _chat_that_can_ask(db, user_id)
|
||||||
|
seen: dict = {}
|
||||||
|
|
||||||
|
from lembas.api import chats as chats_api
|
||||||
|
|
||||||
|
original = chats_api.generation_service.answer
|
||||||
|
chats_api.generation_service.answer = (
|
||||||
|
lambda c, i, *, verdict="", answers=None, reason="": seen.update(
|
||||||
|
verdict=verdict, reason=reason
|
||||||
|
)
|
||||||
|
or True
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
client.post(
|
||||||
|
f"/api/chats/{chat_id}/interaction/p1",
|
||||||
|
data={"verdict": "deny", "reason": "not on production"},
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
chats_api.generation_service.answer = original
|
||||||
|
|
||||||
|
assert seen == {"verdict": "deny", "reason": "not on production"}
|
||||||
|
|
||||||
|
|
||||||
|
def test_a_reason_never_travels_with_a_permission(client, db, registered, user_id):
|
||||||
|
"""Typed into the box, then Allow pressed instead. Reading it there would put
|
||||||
|
the reader's "no, because…" onto a call that went ahead."""
|
||||||
|
chat_id, _message_id = _chat_that_can_ask(db, user_id)
|
||||||
|
seen: dict = {}
|
||||||
|
|
||||||
|
from lembas.api import chats as chats_api
|
||||||
|
|
||||||
|
original = chats_api.generation_service.answer
|
||||||
|
chats_api.generation_service.answer = (
|
||||||
|
lambda c, i, *, verdict="", answers=None, reason="": seen.update(
|
||||||
|
verdict=verdict, reason=reason
|
||||||
|
)
|
||||||
|
or True
|
||||||
|
)
|
||||||
|
try:
|
||||||
|
client.post(
|
||||||
|
f"/api/chats/{chat_id}/interaction/p1",
|
||||||
|
data={"verdict": "allow", "reason": "I changed my mind"},
|
||||||
|
)
|
||||||
|
finally:
|
||||||
|
chats_api.generation_service.answer = original
|
||||||
|
|
||||||
|
assert seen == {"verdict": "allow", "reason": ""}
|
||||||
|
|||||||
Reference in New Issue
Block a user