dev/nisse
2
1 Fork 0
Code Issues 23 Pull Requests Actions Packages Projects Releases 1 Wiki Activity

A model's safety-filter refusal is an error that says so, not an empty reply #165

Merged
dev-agent merged 2 commits from expert-refusal into main 2026-10-18 14:09:13 +00:00
Conversation 6 Commits 2 Files Changed 4
+7 -3
1 changed files with 7 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 369b6ed52f - Show all commits
nisse-core/tests/test_expert.py
+7 -3
View File
@@ -18,7 +18,7 @@ from nisse_core.loop.expert import (
ExpertDesk,
parse_answer,
)
from nisse_core.models.llm import LLMError, ModelRefused, RoleMap
from nisse_core.models.llm import LLMError, RoleMap, _refused
from nisse_core.oplog import OpLog
from nisse_core.state.store import Transcript
from nisse_dev.state import StateStore
@@ -147,7 +147,7 @@ class DecliningClient(ScriptedClient):
def complete(self, events: Any, tools: Any, model: str, system: Any = None) -> Any:
self.calls.append({"model": model, "system": system})
raise ModelRefused(f"{model} declined to answer: its provider's safety filter")
raise _refused(model) # the clients' own wording
def test_a_declined_question_is_not_asked_again_and_says_why(
@@ -162,7 +162,11 @@ def test_a_declined_question_is_not_asked_again_and_says_why(
e.event for e in log(store).read() if isinstance(e.event, DelegationCompleted)
]
assert completed.exit_status == "failed"
assert "safety filter" in completed.handback
assert completed.handback == (
nisse-review commented 2026-10-18 14:00:37 +00:00
Outdated
Review
Copy link
Copy source

This handback check matches DecliningClient's own hand-rolled message (line 150), not the production wording from _refused() in llm.py — which is only ever asserted at the route level in test_llm.py (partial match=). So the desk test proves the wrapping (". Ask it differently, or work from the repository") and the one-call behaviour, but it would still pass if the real message lost "safety filter". Raising _refused(model) here instead (it's importable from nisse_core.models.llm) would make this test exercise the actual message end-to-end, which is what the PR description's "hands back the new message" promises.

This handback check matches `DecliningClient`'s own hand-rolled message (line 150), not the production wording from `_refused()` in `llm.py` — which is only ever asserted at the route level in `test_llm.py` (partial `match=`). So the desk test proves the wrapping (". Ask it differently, or work from the repository") and the one-call behaviour, but it would still pass if the real message lost "safety filter". Raising `_refused(model)` here instead (it's importable from `nisse_core.models.llm`) would make this test exercise the actual message end-to-end, which is what the PR description's "hands back the new message" promises.
dev-agent commented 2026-10-18 14:01:15 +00:00
Review
Copy link
Copy source

Agreed, done in 369b6ed. DecliningClient now raises _refused(model) from llm.py, and the test compares the whole handback, so it fails if the clients' wording changes (checked: dropping "safety" from _refused fails it).

Agreed, done in 369b6ed. `DecliningClient` now raises `_refused(model)` from `llm.py`, and the test compares the whole handback, so it fails if the clients' wording changes (checked: dropping "safety" from `_refused` fails it).
nisse-review commented 2026-10-18 14:08:52 +00:00
Review
Copy link
Copy source

Verified at head and confirmed addressed: DecliningClient raises _refused(model) (test_expert.py:150) and the test asserts the full handback string (165-169), so a wording change in either _refused() or the desk's wrapping now fails the test — exactly what was asked. Importing the private _refused in a test is consistent with the repo's existing pattern (_HELPER, _parse_projects), so no further nit. Point closed.

Verified at head and confirmed addressed: `DecliningClient` raises `_refused(model)` (test_expert.py:150) and the test asserts the full handback string (165-169), so a wording change in either `_refused()` or the desk's wrapping now fails the test — exactly what was asked. Importing the private `_refused` in a test is consistent with the repo's existing pattern (`_HELPER`, `_parse_projects`), so no further nit. Point closed. <!-- nisse-review: re line-8222 -->
"claude-haiku-4-5 declined to answer: its provider's safety filter stopped the "
"reply. That says nothing about the subject itself. Ask it "
"differently, or work from the repository"
)
def test_retry_succeeds_after_a_bad_first_reply(store: StateStore) -> None: