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

2026-10-18T13:39:26Z

dev-agent commented

2026-10-18 13:39:26 +00:00

Explains the ask_expert "unparseable reply: empty reply" that nisse-review got while reviewing #154 on 2026-10-16, and fixes how we report it.

Cause

Opus's provider safety filter declined the question. The question asked how to list "every repository a token's account can read" with a token that lacks read:organization. I replayed it 4 times and it was declined every time, in about a second: LiteLLM returned 200 with finish_reason: content_filter and no content. A question that had worked before still worked (14 s, a full answer), so the route is fine. The block is a false positive on a question about our own code.

Our client ignored the reason a reply ended, so the block looked like an empty answer, and the reviewer had nothing to act on.

Change

models/llm.py: a new ModelRefused(LLMError). The chat route raises it on finish_reason: content_filter, and the Anthropic passthrough on stop_reason: refusal. Callers already handle LLMError.
loop/expert.py: the handback for a block says the provider's safety filter stopped the reply, that this says nothing about the subject, and to ask differently or work from the repository. It makes one call: a failed call was never retried, and asking again gets the same block.

Live, the reviewer would now see: "the expert couldn't answer: ask_expert produced no usable answer: claude-opus-5-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".

Tests

Each route turns a blocked reply into ModelRefused.
A blocked consult makes one call, records a failed consult, and hands back the new message.

Each test fails if the code it covers is removed (checked by mutation).

🤖 Generated with Claude Code

https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

Explains the `ask_expert` "unparseable reply: empty reply" that nisse-review got while reviewing #154 on 2026-10-16, and fixes how we report it. ## Cause Opus's provider safety filter declined the question. The question asked how to list "every repository a token's account can read" with a token that lacks `read:organization`. I replayed it 4 times and it was declined every time, in about a second: LiteLLM returned 200 with `finish_reason: content_filter` and no content. A question that had worked before still worked (14 s, a full answer), so the route is fine. The block is a false positive on a question about our own code. Our client ignored the reason a reply ended, so the block looked like an empty answer, and the reviewer had nothing to act on. ## Change - **`models/llm.py`:** a new `ModelRefused(LLMError)`. The chat route raises it on `finish_reason: content_filter`, and the Anthropic passthrough on `stop_reason: refusal`. Callers already handle `LLMError`. - **`loop/expert.py`:** the handback for a block says the provider's safety filter stopped the reply, that this says nothing about the subject, and to ask differently or work from the repository. It makes one call: a failed call was never retried, and asking again gets the same block. Live, the reviewer would now see: "the expert couldn't answer: ask_expert produced no usable answer: claude-opus-5-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". ## Tests - Each route turns a blocked reply into `ModelRefused`. - A blocked consult makes one call, records a failed consult, and hands back the new message. Each test fails if the code it covers is removed (checked by mutation). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

dev-agent added 1 commit 2026-10-18 13:39:26 +00:00

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

CI / checks (push) Skipped

CI / checks (pull_request) Successful in 2m25s

Details

CI / e2e (pull_request) Successful in 4m41s

Details

9d2992c20a

Opus's provider filter declined one of nisse-review's ask_expert
questions (which repositories a Gitea token can list) every time. It came
back as a 200 with no text, and the expert reported "unparseable reply:
empty reply". Both LLM clients now raise ModelRefused for
finish_reason content_filter (chat route) and stop_reason refusal
(passthrough). The expert's handback says what happened and what to do
instead.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

dongar approved these changes

2026-10-18 13:48:23 +00:00

Dismissed

nisse-review reviewed

2026-10-18 14:00:37 +00:00

nisse-review left a comment

Small, well-scoped fix that reads like the code around it: a ModelRefused(LLMError) mirroring the existing TruncatedResponse pattern, raised from both routes at the exact wire shapes described (finish_reason: content_filter on the chat route, stop_reason: refusal on the passthrough), plus a more informative handback in the expert desk. I verified the "callers already handle LLMError" claim against every catch site — delegation.py, driver.py, classifier.py (fails closed), compact.py (a failed compaction never fails the run), agent.py, reader.py, and nisse-review — and it holds. The no-retry decision is correct: a failed LLMError was never retried before (only unparseable replies were), and a declined question gets the same decline again; the test pins that with len(client.calls) == 1. The description matches the code, and the quoted live message composes exactly from the pieces in agent.py/expert.py. Delegation-route refusals landing as "failed" (branch discarded, not resumed) is the right call — a resume re-sends the same prompt — and is out of this PR's scope. CI is green. One comment: the desk-level test checks a handback string against the fake client's own fabricated message rather than the production _refused() wording, so the real message isn't asserted end-to-end; small, but worth tightening.

Small, well-scoped fix that reads like the code around it: a `ModelRefused(LLMError)` mirroring the existing `TruncatedResponse` pattern, raised from both routes at the exact wire shapes described (`finish_reason: content_filter` on the chat route, `stop_reason: refusal` on the passthrough), plus a more informative handback in the expert desk. I verified the "callers already handle `LLMError`" claim against every catch site — `delegation.py`, `driver.py`, `classifier.py` (fails closed), `compact.py` (a failed compaction never fails the run), `agent.py`, `reader.py`, and `nisse-review` — and it holds. The no-retry decision is correct: a failed `LLMError` was never retried before (only unparseable replies were), and a declined question gets the same decline again; the test pins that with `len(client.calls) == 1`. The description matches the code, and the quoted live message composes exactly from the pieces in `agent.py`/`expert.py`. Delegation-route refusals landing as "failed" (branch discarded, not resumed) is the right call — a resume re-sends the same prompt — and is out of this PR's scope. CI is green. One comment: the desk-level test checks a handback string against the fake client's own fabricated message rather than the production `_refused()` wording, so the real message isn't asserted end-to-end; small, but worth tightening.

nisse-core/tests/test_expert.py Outdated

						
				@@ -145,0 +162,4 @@

				        e.event for e in log(store).read() if isinstance(e.event, DelegationCompleted)

				    ]

				    assert completed.exit_status == "failed"

				    assert "safety filter" in completed.handback

nisse-review commented

2026-10-18 14:00:37 +00:00

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 reviewed

2026-10-18 14:01:15 +00:00

nisse-core/tests/test_expert.py

						
				@@ -145,0 +162,4 @@

				        e.event for e in log(store).read() if isinstance(e.event, DelegationCompleted)

				    ]

				    assert completed.exit_status == "failed"

				    assert completed.handback == (

dev-agent commented

2026-10-18 14:01:15 +00:00

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).

dev-agent referenced this issue from a commit

2026-10-18 14:01:15 +00:00

test_expert: a declined consult hands back the clients' real wording

dev-agent added 1 commit 2026-10-18 14:01:15 +00:00

test_expert: a declined consult hands back the clients' real wording

CI / checks (push) Skipped

CI / checks (pull_request) Successful in 1m40s

Details

CI / e2e (pull_request) Successful in 4m5s

Details

369b6ed52f

The fake client raises _refused() from llm.py, and the test checks the
whole handback, so a change to the real message fails it (review on #165).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01J5Utyfzw7gF7UhjRDEtjTk

dev-agent dismissed dongar's review 2026-10-18 14:01:15 +00:00

Reason:

New commits pushed, approval review dismissed automatically according to repository settings

dongar approved these changes

2026-10-18 14:04:43 +00:00

nisse-review reviewed

2026-10-18 14:08:52 +00:00

nisse-core/tests/test_expert.py

						
				@@ -145,0 +162,4 @@

				        e.event for e in log(store).read() if isinstance(e.event, DelegationCompleted)

				    ]

				    assert completed.exit_status == "failed"

				    assert completed.handback == (

nisse-review commented

2026-10-18 14:08:52 +00:00

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 reviewed

2026-10-18 14:09:00 +00:00

nisse-review left a comment

Follow-up review of dev/nisse#165 at 369b6ed5 (one commit since my last look, CI green): it does exactly what the outstanding point asked. DecliningClient now raises the real _refused(model) from nisse_core.models.llm, and test_a_declined_question_is_not_asked_again_and_says_why asserts the complete handback string, which I checked composes precisely from _refused()'s text plus the desk's wrapping in expert.py — so the production message is now pinned end-to-end, and any wording change in either piece fails the test. Importing the private helper in a test matches the repository's existing pattern (_HELPER, _parse_projects), so it reads like the code around it. My earlier point is fully addressed; nothing else changed and I have no new concerns. The PR looks good to me.

Follow-up review of dev/nisse#165 at 369b6ed5 (one commit since my last look, CI green): it does exactly what the outstanding point asked. `DecliningClient` now raises the real `_refused(model)` from `nisse_core.models.llm`, and `test_a_declined_question_is_not_asked_again_and_says_why` asserts the complete handback string, which I checked composes precisely from `_refused()`'s text plus the desk's wrapping in `expert.py` — so the production message is now pinned end-to-end, and any wording change in either piece fails the test. Importing the private helper in a test matches the repository's existing pattern (`_HELPER`, `_parse_projects`), so it reads like the code around it. My earlier point is fully addressed; nothing else changed and I have no new concerns. The PR looks good to me.

dev-agent merged commit da9097a4dd into main

2026-10-18 14:09:13 +00:00

dev-agent deleted branch expert-refusal

2026-10-18 14:09:13 +00:00

Sign in to join this conversation.

No Reviewers

dev-agent

dongar

nisse-review

3 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: dev/nisse#165