Conversation
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
main. The rest of that PR had already landed independently while it was open: the reasoning-on-output half via feat: carry assistant reasoning on the generation output too #38 (merged 2.5 h after feat: trace reasoning content and offered tools on generations #37 was opened) and the tool definitions via feat: attach the offered tool definitions to every generation #34.<think>reasoning. pi turns a provider's structuredreasoning_contentdelta into athinkingpart, but some OpenAI-compatible servers stream the reasoning inline as<think>tags in the text content instead — the vLLM default, the DeepSeek distills, and proxies in front of either. pi keeps that text verbatim (pi@0.84.2has no occurrence of<think>anywhere indist), so the reasoning was traced as the answer.splitInlineThinking/extractAnswerAndThinkingnow split it out and feed it through the existingextractThinking, so inline reasoning travels the same path as structured reasoning. Only a leading tag counts, which keeps an answer that merely mentions a<think>tag — a coding agent explaining one — from being rewritten. Structured parts win, so a model emitting both is not traced twice.message_updateonly setcompletionStartTimeon non-emptyextractText(...). A reasoning model streams its thinking before any answer text, so a generation that reasons and then calls a tool without prose got no TTFT at all, and a reason-then-text one reported TTFT ≈ the whole call duration. It now also checksextractThinking(...).Linear: https://linear.app/clickhouse/issue/LFE-16443/pi-plugin-inline-think-reasoning-is-traced-as-the-answer-and-thinking
Fixes LFE-16443
Verification
main'ssrc/index.ts(verified by stashing only that file), so they pin the actual gaps rather than the new code.[inline-think]mock run, and a deterministic[think-tool]run asserting that a thinking-only generation reports a TTFT.--model claude-haiku-4-5:medium), same script and prompts both sides, read back via/api/public/v2/observations:<think>end to end against the mock provider: before — https://cloud.langfuse.com/project/cmt01ylzj016zad0ek3bokien/traces/128ec626e57b7b87ee6c8833db83ada1 (nothinking, raw<think>insidecontent); after — https://cloud.langfuse.com/project/cmt01ylzj016zad0ek3bokien/traces/ee29b23254651afcac144e2ca3d13eb7 (reasoning in its own block,<think>in no span).~/.pi/agent/sessions, 39 of 77 assistant messages carry structured thinking and 6 of those have no text part at all.Why this is a draft
The inline-
<think>split emits{type: "thinking", content}, and that shape currently does not render in the Langfuse trace preview — a regression in the normalized I/O parser, filed as LFE-16438 / langfuse/langfuse#17610. The payload here is correct either way, and TTFT renders today, but the reasoning block only becomes visible once that upstream fix deploys. Keeping this a draft until then so the end-to-end check can be done in the UI rather than only against the API.Not in this change
README.mdis untouched, so the feature list still says only "each thinking block is on the step that made it" — worth a follow-up sentence once the upstream render fix is out.available_toolshalf is deliberately dropped: feat: attach the offered tool definitions to every generation #34 already attaches tool definitions to the generation input, which is both what the UI renders and what the SDK'smaskcovers. feat: trace reasoning content and offered tools on generations #37 put them in metadata, which is flattened per key before export and therefore never masked.readAvailableToolsfalls back to all registered tools whengetActiveTools()returns[], which inverts that PR's own "tool not offered vs not chosen" goal. Not carried over.