feat(threads): prefill the whole thread when a turn is inside one #832

Closed
coilyco-ops wants to merge 1 commit from aos/claude/wy58-thread-prefill into main
Member

Closes #769, built to your comment rather than to the design pass.

The toggle is gone

I don't remember asking for a per channel toggle :p. This doesn't seem like a per channel thing, remove that filter condition

So there is none. Whole-thread prefill applies wherever the turn is inside a thread. That also retires two of the spec's acceptance criteria, the toggle-off byte-identity check and the default-off-at-boot check, since neither has anything left to describe.

Worth knowing, because it follows from removing the toggle rather than from anything you asked for: the staged rollout is gone with it. The spec's plan was ship-off, measure a long thread, then enable per channel. This is on everywhere the moment it merges, so the entire context risk now rests on the two bounds below rather than on your being able to watch one channel first. Flagging it rather than quietly absorbing it - if you want a measure-first step back, the cheapest version is landing this with threadPrefillRunes set low and raising it once you have seen real numbers.

What it does

Outside a thread: unchanged. Same call, same MaxContextMessages, same order. That path is a pure extraction into windowHistory, sharing the transcribe step with the thread path so the two cannot drift.

Inside a thread: pages back to the start of the thread. MaxContextMessages is a window into a channel that never ends; a thread does end, and it is the whole exchange the member is asking inside.

Two bounds, because they fail differently

threadPrefillRunes (32000) bounds what reaches the model. Six tool rounds at the default ToolResultBytes already admit more than this, so it sits in family with what a turn carries today rather than being a number I invented from nothing.

threadPrefillPages (10) bounds the Discord round trips, so one enormous thread cannot spend the rate limit every other channel shares. A short page means the thread's start was reached, which is the ordinary exit.

A page that errors fails the prefill rather than returning what it had. Half a thread presented as a whole one is the failure this feature is against.

Oldest first, and said out loud

Over the bound, oldest go first - the newest exchange is the one the member is in. The reply carries [thread context truncated, N older messages dropped] whenever anything was dropped.

The notice reserves its room before the answer is fitted, the same way the tool-disclosure footer does. A notice that vanishes under load reads as context that was never missing.

Mutation

  • Silent truncation (drop the annotation) - TestADroppedPrefillIsAlwaysAnnotated fails on every count.
  • Newest-first instead of oldest-first - the kept entry is the wrong one.
  • Remove the page bound - the endless-thread test makes 1048576 fetches instead of 10.

Coverage I do not have, stated rather than left to be found

The inThread branch inside History is not covered by a test, because both arms need a live *discordgo.Session. inThread itself is tested directly against cached gateway state, and the pagination and budget logic are tested through seams (pageBack takes a fetcher, boundPrefill takes entries). The non-thread path had no direct test before this change either, and the diff shows its ChannelMessages call and t.limit untouched.

Verification

ward gate green: build, policy-check, vet, test, test-skips, pre-commit. Ten new tests, each run individually.

Reasoning: docs/sirens-echo-thread-prefill.md.

🤖 Generated with Claude Code

Closes #769, built to your comment rather than to the design pass. ## The toggle is gone > I don't remember asking for a per channel toggle :p. This doesn't seem like a per channel thing, remove that filter condition So there is none. Whole-thread prefill applies wherever the turn is inside a thread. That also retires two of the spec's acceptance criteria, the toggle-off byte-identity check and the default-off-at-boot check, since neither has anything left to describe. **Worth knowing, because it follows from removing the toggle rather than from anything you asked for:** the staged rollout is gone with it. The spec's plan was ship-off, measure a long thread, then enable per channel. This is on everywhere the moment it merges, so the entire context risk now rests on the two bounds below rather than on your being able to watch one channel first. Flagging it rather than quietly absorbing it - if you want a measure-first step back, the cheapest version is landing this with `threadPrefillRunes` set low and raising it once you have seen real numbers. ## What it does Outside a thread: unchanged. Same call, same `MaxContextMessages`, same order. That path is a pure extraction into `windowHistory`, sharing the `transcribe` step with the thread path so the two cannot drift. Inside a thread: pages back to the start of the thread. `MaxContextMessages` is a window into a channel that never ends; a thread does end, and it is the whole exchange the member is asking inside. ## Two bounds, because they fail differently **`threadPrefillRunes`** (32000) bounds what reaches the model. Six tool rounds at the default `ToolResultBytes` already admit more than this, so it sits in family with what a turn carries today rather than being a number I invented from nothing. **`threadPrefillPages`** (10) bounds the Discord round trips, so one enormous thread cannot spend the rate limit every other channel shares. A short page means the thread's start was reached, which is the ordinary exit. A page that errors fails the prefill rather than returning what it had. Half a thread presented as a whole one is the failure this feature is against. ## Oldest first, and said out loud Over the bound, oldest go first - the newest exchange is the one the member is in. The reply carries `[thread context truncated, N older messages dropped]` whenever anything was dropped. The notice reserves its room *before* the answer is fitted, the same way the tool-disclosure footer does. A notice that vanishes under load reads as context that was never missing. ## Mutation - **Silent truncation** (drop the annotation) - `TestADroppedPrefillIsAlwaysAnnotated` fails on every count. - **Newest-first instead of oldest-first** - the kept entry is the wrong one. - **Remove the page bound** - the endless-thread test makes 1048576 fetches instead of 10. ## Coverage I do not have, stated rather than left to be found The `inThread` branch inside `History` is not covered by a test, because both arms need a live `*discordgo.Session`. `inThread` itself is tested directly against cached gateway state, and the pagination and budget logic are tested through seams (`pageBack` takes a fetcher, `boundPrefill` takes entries). The non-thread path had no direct test before this change either, and the diff shows its `ChannelMessages` call and `t.limit` untouched. ## Verification `ward gate` green: build, policy-check, vet, test, test-skips, pre-commit. Ten new tests, each run individually. Reasoning: `docs/sirens-echo-thread-prefill.md`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
feat(threads): prefill the whole thread when a turn is inside one
All checks were successful
ci / image-build (pull_request) Successful in 30s
ci / test (pull_request) Successful in 46s
ci / publish-echo-image (pull_request) Has been skipped
ci / publish-observed (pull_request) Has been skipped
d3daab3886
Inside a thread the thread is the conversation, and MaxContextMessages is a
window into a channel that never ends. A thread does end, and it is the whole
exchange the member is asking inside.

Outside a thread nothing changes: same call, same limit, same order. The
non-thread path is a pure extraction into windowHistory, sharing the transcribe
step with the thread path so the two cannot drift.

No per-channel toggle. The design pass specified one defaulting off, and Kai
removed it on the issue: this is not a per-channel thing. So it is on wherever
the turn is inside a thread, which puts the whole weight of the context risk on
the two bounds rather than on a staged rollout.

threadPrefillRunes bounds what reaches the model, and six tool rounds already
admit more. threadPrefillPages bounds the Discord round trips, so one enormous
thread cannot spend the rate limit every other channel shares. A page that
errors fails the prefill rather than returning what it had, because half a
thread presented as a whole one is the failure this is against.

Over the bound the oldest go first and the reply says so:
`[thread context truncated, N older messages dropped]`. The notice reserves its
room before the answer is fitted, like the tool-disclosure footer, because a
notice that vanishes under load reads as context that was never missing.

Mutation: dropping the annotation fails TestADroppedPrefillIsAlwaysAnnotated on
every count. Dropping newest-first instead of oldest-first keeps the wrong
entry. Removing the page bound runs 1048576 fetches instead of 10.

Closes #769

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Kai Siren <coilysiren@gmail.com>
Co-authored-by: Claude <noreply@anthropic.com>
Author
Member

Closing this as duplicate work. My mistake, and worth recording rather than deleting quietly.

#827 landed the same feature at 17:13Z. I branched from main at 17:07Z and did not re-check main before pushing, so I built a second implementation of #769 in parallel with one already in flight. CI on this branch was green, but it conflicts with main because both add internal/community/threadprefill.go and docs/sirens-echo-thread-prefill.md.

#827's implementation is the better one and stays. Two things it has that mine did not: the Capped floor, so a thread longer than the walk reports at least N rather than a count it cannot support, and the annotation as a real service-authored suffix contending in the send budget rather than a notice appended outside it.

The actual remaining work on #769 was Kai's comment, not the build:

I don't remember asking for a per channel toggle :p. This doesn't seem like a per channel thing, remove that filter condition

That is #833, which removes the toggle from what #827 landed.

How I would have caught this: re-fetching origin/main before opening the PR, not just before branching. A merged sibling PR closing the same issue is the signal, and git log origin/main --grep '#769' would have shown it in one command.

Closing this as duplicate work. My mistake, and worth recording rather than deleting quietly. #827 landed the same feature at 17:13Z. I branched from `main` at 17:07Z and did not re-check `main` before pushing, so I built a second implementation of #769 in parallel with one already in flight. CI on this branch was green, but it conflicts with `main` because both add `internal/community/threadprefill.go` and `docs/sirens-echo-thread-prefill.md`. #827's implementation is the better one and stays. Two things it has that mine did not: the `Capped` floor, so a thread longer than the walk reports `at least N` rather than a count it cannot support, and the annotation as a real service-authored suffix contending in the send budget rather than a notice appended outside it. The actual remaining work on #769 was Kai's comment, not the build: > I don't remember asking for a per channel toggle :p. This doesn't seem like a per channel thing, remove that filter condition That is #833, which removes the toggle from what #827 landed. **How I would have caught this:** re-fetching `origin/main` before opening the PR, not just before branching. A merged sibling PR closing the same issue is the signal, and `git log origin/main --grep '#769'` would have shown it in one command.
Author
Member

Correction to the comment above: the follow-on is #834, not #833. I wrote the number before opening it.

Correction to the comment above: the follow-on is **#834**, not #833. I wrote the number before opening it.
coilyco-ops closed this pull request 2026-08-15 17:28:43 +00:00
All checks were successful
ci / image-build (pull_request) Successful in 30s
ci / test (pull_request) Successful in 46s
ci / publish-echo-image (pull_request) Has been skipped
ci / publish-observed (pull_request) Has been skipped

Pull request closed

Sign in to join this conversation.
No reviewers
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
coilyco-gaming/sirens-echo!832
No description provided.