fix: ensure thinking blocks in assistant messages for extended thinking

[bigboateng]

Important

Fixes 400 error by ensuring assistant messages start with thinking blocks when extended thinking is enabled.

• Behavior:
  • Fixes 400 error by ensuring all assistant messages start with thinking blocks when thinkingBudget is enabled in samplingLoop() in loop.ts.
  • Adds ensureThinkingBlocksForExtendedThinking() to message-processing.ts to filter out non-compliant messages.
• Functions:
  • Integrates ensureThinkingBlocksForExtendedThinking() into samplingLoop() in loop.ts to enforce message compliance.
• Misc:
  • Adds changeset file fix-extended-thinking-blocks.md to document the patch.

^{This description was created by for d1e5842. You can customize this summary. It will automatically update as commits are pushed.}

[ellipsis-dev]

Caution

Changes requested ❌

Reviewed everything up to d1e5842 in 2 minutes and 18 seconds. Click for details.

• Reviewed 89 lines of code in 3 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. loop.ts:223

• Draft comment:
  Confirm that using !!thinkingBudget (which treats 0 as false) is the intended behavior for enabling extended thinking.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. utils/message-processing.ts:279

• Draft comment:
  Dropping entire assistant messages that lack a thinking block may cause loss of context. Consider prepending a default thinking block instead if preserving history is important.
• Reason this comment was not posted:
  Comment did not seem useful. Confidence is useful = 40% <= threshold 50% The comment suggests a change in behavior regarding how assistant messages are handled. It doesn't ask for confirmation or testing, but rather suggests an alternative approach. This aligns with the rules for making code suggestions. However, it does not directly address a specific code issue or provide a clear suggestion for improvement, which makes it more of a general advice rather than a specific actionable comment.

3. utils/message-processing.ts:280

• Draft comment:
  For better traceability, consider logging which assistant messages are removed due to missing thinking blocks.
• Reason this comment was not posted:
  Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% This comment suggests adding logging, which is a code quality suggestion. However, I need to consider if this violates the rules. The comment says "consider logging" which is a suggestion, not a clear requirement. It's about "traceability" which could be seen as speculative - we don't know if traceability is actually needed here. The function is silently removing messages, but that might be intentional. The comment doesn't point to a clear bug or issue, just suggests that logging "might be nice to have." This feels like it's asking the author to consider something rather than pointing out a clear problem. According to the rules, comments should only be made if there is "clearly a code change required" and should not be "speculative." The function is dealing with message processing to prevent API errors, and silently removing messages could make debugging difficult if something goes wrong. Adding logging could be a legitimate code quality improvement that makes the system more maintainable. This might be an actionable suggestion rather than just speculation. While logging could be helpful, the comment uses "consider" which makes it a suggestion rather than a clear requirement. The rules state comments should only be made when there is "clearly a code change required." This is more of a "nice to have" suggestion about traceability rather than identifying a clear problem. Without evidence that lack of logging is causing issues, this is speculative. This comment should be deleted. It's a speculative suggestion about adding logging for "better traceability" rather than identifying a clear bug or required change. The use of "consider" indicates it's optional, which violates the rule that comments should only be made when a code change is clearly required.

Workflow ID: wflow_ye6WCKycRHkN8Ord

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}

[ellipsis-dev]

Consider using Array.filter to remove messages instead of collecting indices and splicing in reverse; also, the check for index !== undefined is redundant.