Studio: preserve durations across reasoning blocks - #7520
Merged
danielhanchen merged 3 commits intoJul 28, 2026
Merged
Conversation
for more information, see https://pre-commit.ci
Member
|
@codex review |
|
Codex Review: Didn't find any major issues. You're on a roll. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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
added a commit
to danielhanchen/unsloth-staging-2
that referenced
this pull request
Jul 28, 2026
A rendered reasoning group can be closed and then reopened: parseAssistantContent coalesces adjacent reasoning parts, so a provider that emits each block as a complete <think>...</think> chunk lands several blocks in one group. The tracker wrote a group's duration once and never revisited it, so such a group froze at its first close and displayed 0 seconds. Measure from the first time an index becomes visible rather than from the last startGroup, and reopen a closed group while its reasoning text is still growing. Gating on growth is what stops the timer running on into the answer. A duration supplied by the server is now recorded as authoritative so local timing cannot overwrite it. Also fill indices that a single delta skips. startGroup(n) could jump past earlier indices and leave array holes, which JSON.stringify persists as null; a skipped group became visible and closed inside the same chunk, so it gets a measured zero instead. Test discovery now globs tests/, so a second test file cannot be silently skipped by CI, and tsconfig.test.json puts tests/ under typecheck for the first time.
Member
|
@codex review |
danielhanchen
added a commit
to danielhanchen/unsloth-staging-2
that referenced
this pull request
Jul 28, 2026
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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
added a commit
to Souravrajvi0/unsloth
that referenced
this pull request
Jul 28, 2026
Resolve the reasoning-duration collision: main (unslothai#7520) replaced the single reasoningStartAt/reasoningDuration pair with a per-group tracker and moved extractDeltaText into parse-assistant-content, while this branch made the close-tag classification structural for unslothai#7066. - extractDeltaText keeps main's structuredReasoningContinues and regains the unslothai#7066 neutralization plus the closeOffsets the adapter registers as known wrapper boundaries. - hasUnclosedThinkTag keeps main's semantics but resolves the close structurally, so a quoted or fenced literal </think> no longer stops the timer; the adapter passes the streaming, known-close and resume options. - The reasoning group now starts when raw reasoning ARRIVES and stays open while a split marker is held back, and finishGroup accepts the deferred close instant so an unclosed fence does not bill the answer as thought time.
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
2 participants
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 #7444.
A tool-enabled response can contain more than one reasoning block, such as one before a tool call and another before the final answer. Studio stored only one
reasoningDurationvalue for the whole assistant message, so every completed block displayed the duration of the final block. A response with 2 seconds of reasoning, a tool call, and 5 more seconds of reasoning therefore showed 5 seconds on both blocks.Reproduction
Before this change, both reasoning labels used the last server summary. After this change, each label keeps the duration of its own reasoning pass during streaming and after reload.
Fix
reasoningDurationsarray while retaining the final scalar value for older messages and consumers.Tool execution, sampling, and model output are unchanged.
Verification
reasoningDurations: [2, 5], and both labels remained correct after a page reload without another completion request.