Studio: compact a chat by resetting the epoch, not by trimming it forever by danielhanchen · Pull Request #9162 · unslothai/unsloth · GitHub
Skip to content

Studio: compact a chat by resetting the epoch, not by trimming it forever - #9162

Merged
danielhanchen merged 260 commits into
mainfrom
feat/checkpoint-compaction
Aug 20, 2026
Merged

Studio: compact a chat by resetting the epoch, not by trimming it forever#9162
danielhanchen merged 260 commits into
mainfrom
feat/checkpoint-compaction

Conversation

@danielhanchen

@danielhanchen danielhanchen commented Aug 18, 2026

Copy link
Copy Markdown
Member

Stacked on #9161, which is stacked on #9074. Review those first; this PR's diff against #9161 is the last commit only.

Why

The rolling context window trims the oldest turn groups, then trims a little more on almost every reply. Measured on one 12 turn thread, its boundary moved eight times: eight prefix cache breaks, and a conversation that quietly forgets a bit more each turn.

Correction, measured after this description was written: compaction frequency is not where the win is, and on document-sized turns it goes the other way. See the follow-up comment for the numbers. An epoch only accumulates turns that are small relative to the window; when one turn is a large fraction of it, an epoch lasts one turn. What the measurement does support is recall across epochs (4 of 4 planted codes against 1 of 4, and the first epoch's needle still reachable from the last) and the standing-instruction result below.

What it forgets is not recoverable by retrieval alone. In the same evidence campaign, a standing instruction ("always end with STATUS::...") was archived, recalled as four passages, and still not obeyed, while the identical instruction left in plain view was obeyed every time. #9074 and #9161 make retrieval much better and still do not fix that case, because the problem is not which chunks come back, it is that a rule the model must follow is being treated as a passage to look up.

What this changes

Compaction becomes an event rather than a slope. When the next turn will not fit, the model's context resets to

[system prompt + X] + [the newest user turn]

and everything before it stays reachable through search_conversation.

X is a bounded verbatim record of the user's own standing instructions from the turns being dropped, capped at min(1024, 10% of the prompt budget) and at 8 items. It is deterministic: no model call, so there is no summariser to fail. That failure mode is not hypothetical. Other harnesses have shipped compaction whose failed summary silently replaced user messages with a placeholder, and whose failure left the session oversized so every later message re-triggered it.

Nothing is stored. The client re-sends the whole active branch on every request, so X is recomputed from the evicted turns each time, exactly as the sticky boundary already is. No new table, no new client contract. A generated summary would need durable state; this does not.

X goes in the system message rather than a synthetic turn: it is protected from eviction by construction, it needs no chat template support, and standing rules that are not system content are precisely what compaction folds away. The block states that it is a lossy record of earlier conversation and not new policy, and delimiter-like text inside it is escaped, because promoting a user's words into the system role is an authority confusion risk.

Two refusals

Both are about not lying to the user.

  • A reset is only allowed when the dropped turns are archived. Making history unreachable while the notice says it is searchable is the one outcome this must never produce. Incognito, API only and threadless requests keep the rolling window.
  • A reset is only allowed when the model can actually be offered search_conversation. A template that cannot render tools would be handed a memory it can never reach.

Around the reset

  • The epoch already in force is replayed before a new one is considered. Without that the reset repeats every request and evicts the epoch's own first turn, which is a window of exactly one turn, not an epoch.
  • search_conversation is admitted on a compacted thread even with the user's tools off, and admitted alone. A model that meets a large catalogue at a compaction boundary has been observed calling a tool name it guessed rather than the one that exists, and a one tool surface removes the guess.
  • The automatic recall fires on the first turn of an epoch only, gated on a relevance floor (off by default, and it keeps lexical only hits so exact identifier matches are never gated on a similarity they do not carry), and is skipped entirely when the triggering message is a nudge with no earlier instruction to search for instead. Priming the model's first sight of the tool with a search for the word "continue" teaches the wrong lookup.
  • The nudge tells the model that retrieval ran on this turn and that later turns are its own to search for.

UNSLOTH_CONTEXT_POLICY=rolling reproduces the previous behaviour byte for byte: the A/B arm, and the escape hatch if a template family misbehaves. context_window.py is untouched; the reset is expressed through the existing truncate_oldest_messages primitive.

Evidence

Re-ran the compaction campaign with both halves carrying #9074 and #9161, so the only thing that moves between them is the context policy. Model unsloth/Qwen3.5-4B-MTP-GGUF UD-Q4_K_XL at 16,384 context, real runs, screenshots in the comments.

case BEFORE (rolling) AFTER (checkpoint)
standing instruction trims 28 of 38 messages, 6,180 prompt tokens, 4 chunks recalled, no marker resets 38 of 38, 195 prompt tokens, X = 806 chars, emits the marker and the required table
instruction revised, then both evicted trims 54 of 64, 6,101 prompt tokens, no marker resets 64 of 64, 257 prompt tokens, X = 1,059 chars, emits OMEGA and not the superseded ALPHA

The second row is the stronger result: the reply carries the later instruction's marker and not the earlier one, which is the block's oldest first supersession rule choosing correctly between two rules rather than just carrying one. The marker never appears anywhere in the live window on either side, so neither pass can be imitation, and the control thread (same instruction in plain view, no compaction) was obeyed on both sides, so the runs measure compaction rather than the model.

Tests

21 new tests in test_checkpoint_compaction.py covering the reset shape, the epoch, both refusals, the cap, the oldest first supersession order, the escaped delimiters, the irreducible request and the 80 character substantive floor, which is a real bound recorded rather than left to be discovered.

Full backend suite: 26,056 passed on this branch against 26,034 on its base, with byte identical sets of pre-existing failures (43 failed, 15 errors on both, all in diffusion, video, transformers version and a rich help width assertion).

alkinun and others added 30 commits August 16, 2026 12:03
…window

# Conflicts:
#	studio/backend/tests/test_gguf_completion_usage.py
Rolling context eviction currently drops turns with no way for a caller to
learn which ones went. Extract the turn grouping so the eviction unit is
reusable, add an identity-based diff of what a fit removed, and let a caller
reserve room for content it intends to add back after fitting.

The reserve deliberately does not affect whether trimming happens at all, only
how far it goes once it is already required, so a conversation that fits today
is still returned untouched.
An evicted turn is currently gone for the rest of the session, so the model
will state the conversation began wherever its visible context begins. Keep the
turns the rolling window drops in a searchable scope built on the existing
store, chunker, embedder and hybrid retrieval.

The archive is cumulative: every compaction adds to it and nothing is cleared,
so a later compaction can still find what an earlier one evicted. It lives in
its own scope rather than the thread's document scope, because thread documents
are injected in full on every request and would re-inject the whole history.

Idempotent by content hash, since the same turns are evicted again on every
later request. Every entry point degrades to a no-op rather than raising.
Given only a search tool, a model decides for itself whether to look, and
mostly does not. Measured on MRCR v2, a 35B declined on 56% of rows, scoring
0.099 when it skipped against 0.461 when it searched. Forcing one retrieval on
the compaction turn took tool-only 0.258 to 0.604, and the model then called
the tool on 0% of rows, so the common path costs nothing extra.

Recall fires at most once per request, since the tool loop refits on every
iteration. The tool loop renders it as an ordinary tool exchange through the
builder shared with document auto-inject; the plain path prefixes the latest
user message instead, because it sends no tools array and a tool role without
one breaks strict chat templates.

Only scalar counts join the context_truncated event, so no message content
reaches the wire.
Forced recall answers the turn that evicted, but a later turn can refer to
something the forced pass had no reason to fetch. Add search_conversation so
the model can go looking, scoped to the thread's own archive and sharing the
admission slot with document search so the two cannot race for the embedder.

The tool is offered only once a thread has actually had turns archived, so an
ordinary short chat never pays for the schema, and it is classified read-only
so auto mode does not prompt on every call. A matching system-prompt note tells
the model the session was compacted, since otherwise it assumes the
conversation began where its visible context begins.

Deleting a thread now drops its archive rather than leaking a scope per chat.
_select_request_tools also serves the token-count request model, which has no
thread_id field, so the archive gate raised there.
Recalling your own conversation is mostly an exact-match problem: a name, a
number, an identifier someone pasted twenty turns ago. Those live or die on
rare-token matching, and hybrid fusion was losing them.

Measured on a 30-turn walkthrough of a 230k-character document at a 16k window,
where every turn shared the same wrapper text. The chunk holding the needle
ranked 3rd lexically at any k, was never returned by dense retrieval at all,
and RRF pushed it to 16th because it had 30 useless dense hits to fuse with.
End to end the model answered with the exact code once lexical leads, and could
not answer at all before.

Dense still fills whatever the lexical pass leaves, for paraphrased recall.
Editing an earlier message rewinds a thread and continues down a new branch,
but the archive is append-only and still holds everything the abandoned
continuation produced. Verified against a live build: after rewinding past a
turn, querying its distinctive text still returned it, so the model could be
handed a turn that on this branch never happened.

Recall now drops archived turns that are absent from the thread's saved
transcript. Threads with no saved transcript are left unfiltered, since an API
caller may pass a thread_id without persisting messages and an empty transcript
is absence of evidence rather than evidence the turns are gone.

Containment on a normalised prefix rather than a digest, because the archived
copy is rendered from the inference projection and the saved copy comes back
through the message store.
The only signal that a long chat had been compacted was a toast, which vanishes after
a few seconds and does not survive a reload. A user who scrolls back later has no way
to find out why the model seemed to forget the start of the conversation.

The notice renders from metadata.custom.contextTruncation inside the assistant
message's own container, so it is not part of the conversation sent to the model, is
not editable, and is not exported as content, but it stays attached to the turn it
describes. It reports how many messages were dropped and, when the conversation
archive is on, that they are still searchable and how many passages were recalled.
A thread that has outgrown its window compacts on every turn from then on, not
just the first, so a notice per compacted turn was a notice on every reply for the
rest of the conversation. The user needs telling once.

The notice is now gated on being the first compacted assistant turn in the thread,
found by walking the thread rather than assuming the compacted turns are contiguous
or that this is the last one, so a rollback that removes the turn it was on moves it
to whichever turn now compacts first. The wording follows: it describes the state
the conversation is in from here on, and carries the counts from the turn it began
on in parentheses.
The fit was stateless. The client re-sends the whole saved transcript on every
request, so "keep the newest N tokens" recomputes from scratch each time and slides
forward a turn or two at a time. Measured on a 40-turn thread against an 8k window,
the eviction boundary moved on 12 of 40 turns, which means every few replies quietly
lost a little more of the conversation, llama-server's prefix cache was thrown away
each time the head of the prompt moved, and there was no such thing as a compaction
event to tell the user about.

Two changes make it discrete. The fit now reads back the boundary the thread last
compacted to, from the newest assistant turn's own persisted truncation, and reapplies
it before deciding anything, so nothing new is stored and it survives a restart. And
when the boundary does have to move, the trim takes a further ROLLING_COMPACTION_
HEADROOM_RATIO of the budget out (default 0.25) rather than skimming to the brim, so
the new boundary has room to stay put. Same 40-turn thread, same window: 4 compactions
over 60 turns instead of 14, keeping about 82 percent of the usable budget.

Both are gated on the prompt not already fitting, exactly as the recall reserve is. A
conversation inside its window is never evicted to satisfy either, and a stale boundary
from a branch that was rolled back cannot evict a chat that now fits.

The notice follows: it is shown when dropped_messages rises above the last turn that
reported it, so it appears once per compaction and stays quiet in between.
The notice is a sidecar rendered from metadata, not a message, and the ways that
could quietly stop being true are all one careless edit away: moving it inside
MessagePrimitive.Parts would make it a content part, and everything that walks parts
would then replay it to the model, copy it and export it.

Asserts it renders as a sibling of the content parts, that neither the outbound
message builder nor the assistant replay serialiser reads the key it renders from
(bounded to those function bodies, since the streaming handler reads the same key
legitimately on the way in), that the markdown export does not mention it, and that
it is suppressed while editing so it cannot be typed into the textarea and saved
back as text.
If the message just sent is itself bigger than the window, no amount of eviction
helps. The fit already handled that correctly, returning the conversation untouched
so the request reaches llama-server's normal context-length error, but what the user
was then told was actively misleading: the error reports the size of the WHOLE
conversation and advises shortening it, when the history has already been evicted and
the single message is the part that does not fit. Measured at a 4096-token window: a
5000-token message produces 'Message too long: 10290 tokens ... shorten the
conversation', and shortening it cannot possibly work.

The fit now returns a fits:false diagnosis instead of a bare None, carrying what the
conversation could not be reduced below and how much of that is the latest turn.
Every consumer already gated on fits, so this is inert wherever a truncation is
treated as a compaction; the streaming paths now forward it so the client can use it.

The toast reads the diagnosis and, when the latest turn alone exceeds the window,
says so with the numbers instead of offering advice that leads nowhere. The merge
drops the diagnosis once a later refit succeeds, by delete rather than by assigning
undefined, so an ordinary response keeps exactly the shape it had before.
Fifteen items, each reproduced against the code before changing anything.

Correctness in the request path. The fit reported a successful recall-capable fit
whenever the result was under the prompt budget, but protected messages can stop the
trim reaching the reserve target, and the recall then went in anyway: reproduced at
ctx 8000, the fit accepted at 6900 and recall took the request to 8948, past the
window it had just been made to fit. Recall is now sized from the room the fit
actually obtained, and skipped when there is none. Separately, the sticky boundary
describes the original transcript, so re-applying it on a later tool-loop fit evicted
another boundary-sized block of live history: measured, a second fit dropped 28 of the
30 surviving messages instead of 14, and the summed count persisted an inflated
boundary for the next request. It is now spent after the first fit of a request.

Availability and privacy. enabled() trusted RAG_AVAILABLE, which only records that
import sqlite_vec worked; rag_available() exists because the native vec0 library it
loads is a separate file a venv can lack. On such a machine the fit held a recall
reserve back, evicting extra history, and then both the archive write and the recall
failed, so the user paid for content they never got. And a temporary chat is never
written to studio.db, yet the frontend still sends its thread_id and the request
carries no incognito flag, so its turns were archived to a scope no deletion flow
could reach. Archival now requires the thread to be persisted, which is the same rule
that keeps every archive reachable by a delete. Clear-history and project deletion
drop archives too; only DELETE /threads did.

Archive integrity. The document was committed before its chunks, so a failed chunk
write left an empty row marked completed that document_by_hash then skipped forever;
both now go in one transaction. The live-branch filter accepted a turn on its first
matching line, so editing only the assistant half kept serving the old answer; the
whole turn must be present. Retrieval fetched exactly k before that filter, so stale
turns could starve live ones and recall returned nothing; it over-fetches first. Tool
turns archived only the tool name, which cannot answer what was actually run, so a
bounded rendering of the arguments and the assistant text goes in as well.

The tool surface. Studio always sends an explicit enabled_tools array and has no
reason to name an internal tool, so the allowlist filter removed search_conversation
before the archive gate ran and the tool, plus the compaction nudge gated on it, never
appeared in a Studio chat. It now follows the archive rather than the allowlist. The
forced recall rendered a tool exchange even when the tool was absent from the
catalogue, which is the strict-template hazard the plain path avoids; it picks inline
in that case, and always inline for the final-answer request, which sends no tools at
all. A model-supplied top_k reached a slice as out[:-1] and returned nearly the whole
candidate pool, so it is clamped. Both retrieval tools now share the per-turn search
cap; only the knowledge-base one was counted.

The UI. A fits:false diagnosis is the fitter reporting it could NOT fit, so toasting
that older turns were removed was untrue and burned the once-per-thread flag a later
real compaction needed. The too-long advice compared the latest turn against the raw
context length rather than the prompt budget, so a 3,500-token message in a
4,096-token window was still told to start a new chat, which fails identically.
Four items, three of them about last round's own fixes.

Archived tool turns had become permanently unrecallable. render_turn now writes
'assistant called X: args' and 'tool result: ...' lines, while assistant-ui persists a
tool call as a structured tool-call content part that the transcript flattener dropped;
with every archived line required to appear in that transcript, no tool turn could ever
match. The transcript now flattens toolName, args and result, and the probe strips the
'assistant called <name>:' label, which is ours rather than the stored message's.

The branch probe compared only the first 160 normalized characters of each line, so an
edit to the tail of a long answer left the stale copy eligible. Ordinary lines are now
compared whole; tool results keep a prefix, since render_turn deliberately truncates
those and the archived copy is not meant to equal the stored one.

The forced recall sized itself by dividing the remaining budget by CHUNK_TOKENS, which
is an embedding-token limit rather than the chat template's cost, and prices none of
the wrappers around the injection. It is now recounted with the same tokenizer the fit
used and dropped if it overshoots, so the estimate can no longer eat the reply reserve.

An omitted top_k on search_conversation defaulted to the clamp ceiling of eight rather
than the configured recall default, so an ordinary search could return eight archived
turns into the protected current exchange that rolling truncation cannot evict.
A thread's stored rows are the whole message DAG. Retry and regenerate keep the
replaced response as a sibling on purpose, so filtering recall against the whole
thread cannot tell a live turn from one the user replaced, and an archived copy of
the abandoned response could be recalled into a branch where it never happened.

Filter against the messages the request was actually sent with instead, which is
one branch by construction, and hand the same branch to search_conversation so the
model cannot ask for what the forced recall refused. Falls back to the thread-wide
blob for a caller with no branch to offer.

Also retunes two respawn-refit fixtures whose windows no longer produced two
compactions after compaction started trimming a headroom margin below the budget.
…ctive branch

Deleting a chat cancels its generation, but cancellation is cooperative and the
chunk-and-embed pass between the archive's liveness check and its commit does not
observe it. A delete landing in that window drops the thread's rows and sweeps its
scope before the commit puts rows back, leaving content the user deleted in a scope
no later delete can reach. Re-check after the commit and drop the scope: the delete
route removes rows first and sweeps archives last, so either order converges.

The sticky compaction boundary had the same thread-wide read as recall did. The
stored rows are the whole DAG ordered by creation time, so after a Retry the newest
assistant turn can be the sibling the user switched away from, and its boundary is
sized for history the active branch does not have. Resolve it against the request's
own messages instead.
search_conversation is advertised by thread, not by backend: the tool selector is
shared, so a chat compacted under a GGUF model still offers it after the user
switches to a safetensors one. The safetensors loop had neither guard the GGUF loop
applies to it. It passed no active branch, so a search there fell back to the
thread-wide rows and could answer from a branch Retry left behind, and it capped
only search_knowledge_base, so paraphrased conversation searches could append
archived passages into the protected current exchange on every iteration until the
window failed. The shared set of capped retrieval tools now lives beside the cap.

The forced recall also derived its query from the loop conversation, which on a
later iteration can end with an internal user-role re-prompt rather than anything
the user wrote. It reads the request branch's own latest user turn instead.
…erflowed

One over-fetch is not enough for the live-branch filter. Rewinding or retrying a
continuation that had already been compacted leaves enough stale turns to fill any
fixed candidate window, and the whole page is then rejected while the live match
sitting just below it is never examined, so recall reports nothing although the
answer is in the archive. Widen and re-ask instead, stopping as soon as there are
enough live hits, when the archive stops yielding candidates, or at a bound.

The irreducible-fit diagnosis also carried the size of the last message without
saying whose it was. A tool loop refits with the tool result appended, so that turn
is often output the user never wrote and cannot edit, and the client told them to
shorten it. It now reports the role, and the advice splits on it.
chatgpt-codex-connector[bot]

This comment was marked as resolved.

danielhanchen and others added 4 commits August 19, 2026 23:26
…path

A rewind or a bigger window can put an evicted occurrence back in the prompt, and
re-stamping a copy per transcript seat then left two byte-identical documents, so a
recall slot went on text the model could already read.
A cached yes outlived the moment it described: one request inside the window is enough
to start an epoch, and archive_turns then swallows the write failure while the fitted
prompt has already dropped the turns it says are searchable. The caller memoises per
fit, which is the only span where the answer cannot change under it.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ed8bd20abc

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread studio/backend/routes/inference.py Outdated
Comment on lines +3530 to +3532
for message in rows:
if id(message) not in twin_ids and _checkpointed(message):
return True

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Stop reopening tools after a checkpoint becomes stale

When a checkpointed thread is reloaded with a larger context window so the full branch fits again, fit_checkpoint_context deliberately stops replaying the saved boundary and returns no checkpoint metadata. On the following request, however, the newest assistant row is uncheckpointed while this loop still finds any older checkpoint row and returns True; the route therefore overrides enable_tools=false and enters the Studio tool loop on every subsequent turn even though no compacted epoch remains to repair. Limit this check to the latest distinguishable assistant state so a stale historical reset does not permanently force search_conversation back on.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed and fixed, and my own docstring was the wrong instinct: it said a thread that has checkpointed once keeps saying so. That is right only while the epoch is in force.

The newest distinguishable assistant state decides now, and nothing older. While an epoch holds, every fit records checkpoint on the turn it produced, so a thread mid-epoch still answers yes; once the window grows and the whole branch fits again the fit records nothing, and scanning back to an older reset was forcing the Studio tool loop open for the rest of the thread's life, overriding enable_tools = false and taking the n > 1 and non-streaming guards with it, to repair a compaction that no longer exists.

Test covers both directions: a stored newest turn with no truncation behind an older reset returns False (True before), and both turns checkpointed still returns True.

Separately, the Source lint failure on this PR was mine, not the queue: removing the reachability memo left import time unused in conversation_archive.py. Removed.

Every ordinal is NULL there, so both halves ordered by nothing but the score and
returned the same rows, which the merge then deduplicated into one end's worth of
candidates. Order by created_at in each direction, with the chunk id as a stable last
resort.
An instruction is 80 characters, but a query only has to name something. A thread of
short prompts followed by a nudge had no anchor at all, so the reset left the model the
nudge alone: no recall, no carried block under the same length rule, and no search tool
until the archive existed. Fall back to the last earlier turn that is not thin.
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026
@unslothai unslothai deleted a comment from chatgpt-codex-connector Bot Aug 19, 2026
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Chef's kiss.

Reviewed commit: 37f4d87b4e

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

A reset is a state of the thread now, not a mark it carries for ever: once the window
grows and the whole branch fits again the fit records nothing, and scanning back to an
older reset forced the tool loop open for the rest of the thread's life. The newest
distinguishable assistant state answers instead.
@danielhanchen

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: ff545a3d8d

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen
danielhanchen changed the base branch from feat/conversation-recency to main August 20, 2026 00:41
Squashed onto main, so every file this branch shares with that PR collides with our own
copy of the same work. Resolved in favour of this branch at every marker, keeping main's
unrelated changes inside those files.
@danielhanchen
danielhanchen merged commit 28b8880 into main Aug 20, 2026
36 of 43 checks passed
@danielhanchen
danielhanchen deleted the feat/checkpoint-compaction branch August 20, 2026 00:59

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 43244c4e24

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +3582 to +3585

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Distinguish repeated replies by branch position

When the newest assistant reply repeats text that appeared earlier on the same linear branch (for example, two separate "Done." replies), twins includes every stored row with that text rather than only indistinguishable siblings at the newest branch position. If any older occurrence predates the checkpoint, all(...) returns false even though the newest state is checkpointed, so the next tools-off request does not reopen the search_conversation loop and the active epoch may be reset again instead of replayed. Preserve occurrence/branch position when resolving duplicates rather than collapsing all matching text together.

Useful? React with 👍 / 👎.

meefs pushed a commit to meefs/unsloth that referenced this pull request Aug 20, 2026
…ard them (unslothai#9353)

* Put back the AppleDouble GGUF filters unslothai#9074 reverted, and guard them

Backend CI's 3.13 leg has six failures on main beyond the two in unslothai#9348. Three
separate causes, all of them in the tests or in a merge resolution rather than
in anything a PR meant to change.

1. llama_cpp.py lost every line of unslothai#8919
------------------------------------------------------------------------
unslothai#8919, "never pick a macOS AppleDouble sidecar as a GGUF", touched 49 files.
18b97f8 ("keep and search the turns rolling context evicts", unslothai#9074) reverted
all five of its hunks in core/inference/llama_cpp.py and nothing else. That is
the signature of a branch cut before unslothai#8919 landed and merged whole-file: unslothai#9074
is a rolling-context PR, its diff carries no replacement for any of this, and it
did not revert the tests, which is the only reason CI said anything at all.

Checked the rest of unslothai#8919 line by line against main: of the 49 files it changed,
llama_cpp.py is the only one that lost anything. All five hunks are restored
here, and the file now contains every line unslothai#8919 added.

Four of the five are the selection sites, and they are the half that was silent:

  _gguf_snapshot_files      a local walk now skips the companion on its bytes
  _pick_mmproj              "._mmproj-F16.gguf" satisfied the F16 preference and
                            sorted ahead of the real adapter
  _pick_dspark              every GGUF under dspark/ qualifies, so a sidecar
                            ranked equal to its sibling and sorted first; also
                            back at module level, where unslothai#8919 put it because it
                            is handed a live repo listing as well as a snapshot
  the HF list_repo_files    a repo listing has no bytes to read

The fifth is the one CI caught: the "invalid magic characters" branch in
_classify_llama_start_failure. Without it a user who points llama-server at a
"._model.gguf" sidecar gets "Check that the GGUF file is valid and you have
enough memory" and goes off to free memory they already have, which is issue
unslothai#8566 exactly. Restored verbatim at its original anchor, below the dyld branch
so test_a_dyld_failure_still_outranks_it keeps holding.

Five new tests in test_appledouble_guards.py cover the four selection sites
behaviourally, including that _pick_dspark is reachable from module scope, since
nesting it back inside the method is how it was reverted. Each also pins that a
file a user genuinely named "._something" still resolves: nothing may be refused
for its name alone.

Mutation-tested by restoring llama_cpp.py to its current main state: 7 failed,
all five new guards plus the two that were already red.

2. The refactor guard baseline
------------------------------------------------------------------------
unslothai#9074 added RAG_SEARCH_TOOLS to core/inference/tool_call_parser, which is one of
the two strict modules the guard runs with additions_matter, so a new public name
there is a deliberate re-baseline by design. The symbol is correct: three modules
import it, and test_conversation_recall_injection.py already pins its value.

Re-baselined through the tool's own `snapshot`, then trimmed to just this entry.
The full snapshot also absorbed 52 unreviewed new names in core.inference.llama_cpp,
3 in safetensors_agentic and 151 lines of patch_targets churn. Those are additive
drift the guard tolerates on purpose, so recording them fixes nothing and pins
symbols nobody looked at.

Mutation-tested: an added throwaway public symbol still turns both tests red, so
the strict-addition behaviour survived the re-baseline.

3. The research opt-out payload
------------------------------------------------------------------------
28b8880 ("compact a chat by resetting the epoch", unslothai#9162) added tools_withheld
to the generation kwargs. test_the_opt_out_changes_nothing_a_default_install_does
compared the two payloads whole, which was right when every kwarg was
model-facing.

tools_withheld is not. Its only consumer is _can_reset_epoch, which picks a
compaction strategy; it never reaches the prompt, the sampling params or the tool
catalogue. And it has to differ: without the opt-out a compacted thread can still
re-admit search_conversation through the checkpoint-repair branch, so resetting
the epoch is safe, while with the opt-out that repair is closed on this turn and
every identical turn after it, so a reset would strand the epoch behind a tool
that never arrives. Forcing the two equal is what would break Deep Research,
which sends a real thread_id.

The 17 model-facing fields are identical and neither side carries a `tools` key,
so the asymmetry this test exists to catch is not present. Rather than just
excluding the key, it is now pinned in both directions plus an explicit
no-tool-catalogue assertion, so the file catches more than before.

Mutation-tested: pinning tools_withheld = False in routes/inference.py fails the
new assertion for both parameters.

Verification
------------------------------------------------------------------------
test_appledouble_guards.py, test_llama_cpp_start_failure_classification.py,
test_refactor_guard.py, test_research_internal_call_tool_gate.py and
hub/tests/test_model_services.py: 508 passed, from 6 failed.
Wider sweep over tests/test_llama*.py and the hub model services: no collateral.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants