ai_analyst: relay what the model is doing while the tools run - #295
Merged
Conversation
| _answer('Nothing notable.')]) | ||
| await _handle(_analyst(llm, poster=poster), '@ai check evil.example.com') | ||
| self.assertTrue( | ||
| any('evil.example.com' in p['text'] and p['props'].get(ai_analyst.PROP_KEY) |
A turn can take minutes on a reasoning model, and until it ended the thread showed nothing: the progress line only fired when a single reply carried more than one tool call, and only once per turn, so the common one-lookup-per-round case posted nothing at all between the question and the answer. Worse, when the model narrated what it was about to do, that text was discarded outright -- handle() went straight to executing the tools and never posted reply['content'] unless the reply had no tool calls. Now every tool round announces: the model's own narration when it gave one, otherwise the lookups being run. Posts are tagged PROP_PROGRESS, which reconstruct() already skips, so none of it becomes the model's memory, charges the tool budget, or can be mistaken for a pivot proposal that a later bare "yes" would approve. That last property is covered by its own regression test. Assertions that counted or indexed poster.posts positionally now select by prop. A progress relay is interleaved with the reply, so a positional index drifts whenever relay volume changes; selecting by kind tests the actual intent. Note this changes what the thread SHOWS, not how long a turn takes. The LLM call is still non-streaming with a scalar requests timeout, so generation longer than AI.timeout still fails on a read timeout, retried once.
spierenburg
force-pushed
the
feat/ai-relay-progress
branch
from
August 23, 2026 20:59
5bcd0ea to
ab71d0d
Compare
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.
Follow-up to #288. An
@aiturn can take minutes on a reasoning model, and untilit finished the thread showed nothing at all.
What was wrong
Two separate gaps:
The progress line almost never fired. The condition was
if not announced and len(tool_calls) > 1— one reply had to carry more than onetool call, and it only ever fired once per turn. The common case is one lookup per
round, so the thread went silent between the question and the answer.
The model's narration was thrown away. When a reply carried tool calls,
handle()went straight to executing them;reply['content']was only ever postedon a reply that had no tool calls. So a model saying "let me pull the certificates
first" alongside its tool call had that text silently discarded.
Measured before the change — the model narrated, and the channel saw only this:
What it does now
Every tool round announces: the model's own narration when it gave one, otherwise
the lookups being run.
Why this is safe
Relays are tagged
PROP_PROGRESS, whichreconstruct()already skips — its existingcomment names progress notes explicitly. So narration never becomes the model's
memory, never charges the tool budget, and never becomes a pending proposal.
That last one is the property worth stating plainly: if the model narrates "next I
should look at 9.9.9.9", that must not become something a later bare
yeswouldauthorize. Posting as progress rather than reply is what prevents it, and
test_relayed_narration_does_not_become_a_pending_proposalpins it down.This deliberately does not relay the model's raw reasoning tokens. Reasoning over
an
<untrusted_tool_result>block routinely quotes it verbatim, so relaying CoT wouldrender attacker-influenced text as Mattermost markdown — the injection class described
in
docs/plans/2026-07-07-command-render-sanitization-adw.md. That wants the rendersanitizer built first, and is a separate change.
Scope note: this does not change turn duration
It changes what the thread shows, not how long a turn takes. The LLM call is still
non-streaming with a scalar
requeststimeout, so generation exceedingAI.timeoutstill fails on a read timeout (retried once, so ~2× the configured value in wall
clock). Deployments running a large reasoning model will want
AI.timeoutwell abovethe shipped 60.
For the record on the surrounding concurrency, since it came up: websocket handling is
unaffected — each message is dispatched via
asyncio.create_task(matterbot.py:329),so a long turn never blocks the message pump. AI tool calls are individually bounded by
asyncio.timeout(self._command_timeout)and return a string rather than raising.Test plan
Three new tests: narration is relayed and tagged as progress; a single-tool round now
announces; relayed narration does not become a pending proposal.
Seven existing tests broke, all because they counted or indexed
poster.postspositionally and a progress post now precedes the reply. Each was verified as a test
shape issue, not a regression, then rewritten to select posts by prop rather than
by index — a positional index drifts whenever relay volume changes, whereas selecting
by kind tests the actual intent.
python -m unittest discover -s tests→ Ran 317 tests, OKruff check --select F821 ai_analyst.py tests/test_ai_analyst.py→ cleanNot verified against a live endpoint.