fix(types): handle reasoning.encrypted content block type from Grok models#1732
fix(types): handle reasoning.encrypted content block type from Grok models#1732potb wants to merge 7 commits intocode-yeongyu:devfrom
Conversation
There was a problem hiding this comment.
3 issues found across 14 files
Confidence score: 4/5
- This PR looks generally safe to merge, but there is a user-facing risk:
reasoning.encryptedparts are filtered out before formatting insrc/tools/background-task/full-session-format.tsandsrc/tools/background-task/modules/formatters.ts, so the new handling is unreachable and content gets dropped even whenincludeThinkingis true. - The test in
src/hooks/thinking-block-validator/reasoning-encrypted.test.tsis tautological and doesn’t exercise production logic, so it won’t catch regressions in the validator. - Pay close attention to
src/tools/background-task/full-session-format.ts,src/tools/background-task/modules/formatters.ts,src/hooks/thinking-block-validator/reasoning-encrypted.test.ts- reasoning.encrypted handling is dropped and test coverage is ineffective.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/hooks/thinking-block-validator/reasoning-encrypted.test.ts">
<violation number="1" location="src/hooks/thinking-block-validator/reasoning-encrypted.test.ts:5">
P2: Test is tautological: it only checks a locally defined helper instead of exercising production logic, so it provides no real coverage of the thinking-block validator.</violation>
</file>
<file name="src/tools/background-task/full-session-format.ts">
<violation number="1" location="src/tools/background-task/full-session-format.ts:138">
P2: `reasoning.encrypted` parts are filtered out before formatting, so the newly added handling is unreachable and the content is discarded.</violation>
</file>
<file name="src/tools/background-task/modules/formatters.ts">
<violation number="1" location="src/tools/background-task/modules/formatters.ts:305">
P2: `reasoning.encrypted` parts are filtered out before formatting, so the newly added handling is unreachable and these parts are dropped even when `includeThinking` is true.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
4aec122 to
d45b87f
Compare
…NKING_TYPES Set Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
…traction Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
…g content extraction Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <[email protected]>
d45b87f to
f634150
Compare
The filter that normalizes message parts before formatting only checked for 'thinking' and 'reasoning', so reasoning.encrypted parts were silently dropped even when includeThinking was true.
|
@cubic-dev-ai recheck |
@potb I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 13 files
Confidence score: 5/5
- Low-severity test quality concern only; no concrete user-facing regression is indicated, so this looks safe to merge.
src/hooks/thinking-block-validator/reasoning-encrypted.test.tsvalidates a local helper instead of production logic, so the test could pass even if the real thinking-family check is wrong.- Pay close attention to
src/hooks/thinking-block-validator/reasoning-encrypted.test.ts- test may not exercise actual production behavior.
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="src/hooks/thinking-block-validator/reasoning-encrypted.test.ts">
<violation number="1" location="src/hooks/thinking-block-validator/reasoning-encrypted.test.ts:5">
P3: This test defines and validates a local helper instead of exercising production logic, so it will pass even if the real thinking-family check is incorrect (e.g., `startsWithThinkingBlock` still ignores `reasoning.encrypted`). It doesn’t provide regression protection for the intended change.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
…thThinkingBlock and findPreviousThinkingContent Address review feedback (identified by cubic): the thinking-block-validator hook was missing reasoning.encrypted in two internal functions, and the test was tautological (testing a local helper instead of production logic). - Add reasoning.encrypted to startsWithThinkingBlock so messages starting with encrypted reasoning are recognized as having a thinking block - Add reasoning.encrypted to findPreviousThinkingContent so encrypted reasoning from prior turns can be recovered - Rewrite test to exercise createThinkingBlockValidatorHook directly with message fixtures instead of a locally defined helper
|
Thank you for the thorough effort across 14 files — this was clearly a lot of careful work! After investigating the OpenCode source code, we found that
The message processor in OpenCode ( We really appreciate you taking the time to look into Grok model compatibility — that kind of cross-model thinking is exactly what we need. Thank you! 🙏 |
Closes #1588
Grok models return
reasoning.encryptedcontent blocks. We only check for"reasoning"and"thinking", so these blocks get silently dropped during validation, extraction, and formatting.Added
reasoning.encryptedto theThinkingPartTypeunion andTHINKING_TYPESset, then updated all downstream checks. Content extraction guards on&& part.textso encrypted blocks without text are safely skipped.