Repository navigation
fix(security): lower thinking effort for security explanations - #470
Draft
johnmatthewtennant wants to merge 5 commits into
Draft
johnmatthewtennant wants to merge 5 commits into
johnmatthewtennant wants to merge 5 commits into
Conversation
A security alert's explanation runs in a hidden session that inherits the user's default thinking effort. At high or max effort the model can think past the 20 s budget before writing text, and the failure is never logged. These tests expect the session to turn thinking off and the failure reason to reach the app log. Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
Set thinking_effort to off on the hidden explanation session after setModel, so the model answers within the 20 s budget. Models without the option keep their default. Log why an explanation failed (no reply text in time, or the error) to the app log instead of discarding it. Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
Some always-on thinking models map off to high at the Goose pin, so off can raise the explanation's effort above a user's low default. Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
Off maps to high on always-on thinking models at the Goose pin, so it could raise effort for users whose default is low or medium. Low is the lowest effort every thinking model honors, and OpenAI already sends low for off. Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Signed-off-by: John Matthew Tennant <jtennant@block.xyz>
Signed-off-by: John Matthew Tennant <jtennant@block.xyz> Co-authored-by: Gordon <6581f171b4f294207c04c83d43b057ea88caa717457980f756160f0212d4f429@buzz.block.builderlab.xyz> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * origin/main: fix(connections): remove retired Block Academy provider (#475)
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This fixes a bug: when a security alert has no detector-written explanation, Berd should generate one, but it can fail with "An explanation could not be generated" and nothing is logged.
inferSecurityExplanationruns the explanation in a new hidden session, and that session inherits the user's default thinking effort. At high or max effort, the model can spend the whole 20 s budget thinking before it writes any text.promptForTextthen returns null, and the catch discards every other error, so the cause never reaches a log.This PR:
thinking_efforttolowon the hidden session aftersetModel(which can rebuild that option) and before the prompt. Notoff: always-on thinking models such as Fable 5 treatoffashigh, so it would raise effort for a user whose default is low. OpenAI reasoning models already sendlowforoff. A model without the option keeps its default, and the explanation still runs.logRendererEvent:security explanation failed: no reply text within 20000 msfor a timeout or empty reply, or the formatted ACP error.Related issue
None found.
Testing
Red/green through the production path: Berd's
inferSecurityExplanationran against the pinned Goosegoose serveover ACP. The model was a local stub that records the effort Goose sends and takes longer than the 20 s budget athigh, so model latency was simulated.offlow(this PR)low(this PR)low(this PR)Live calls with a user default of high, two per model and effort, all returned an explanation inside the budget. At
low, first text arrived in 1.8 to 2.8 s on Opus 5 (6.8 to 8.7 s at inherited high) and 1.8 to 1.9 s on Fable 5 (2.0 to 5.9 s).Unit tests in
inferExplanation.test.tscover the fixed contract:lowis set betweensetModeland the prompt (fails on main and withoff);Not verified: opening an alert without a detector-written explanation in a packaged build. The runs above call
inferSecurityExplanationdirectly rather than going through the alert UI.🤖 Generated with Claude Code