fix: sticky models, Stop, output guards, tool calls, adult mode#8
Conversation
Pin preferred/custom models across history wipe and RAM gates, honor keep-warm idle, bump flutter_gemma to 1.3.1, and stop runaway streams via length/repetition limits plus a Stop control. Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @involvex, your pull request is larger than the review limit of 150000 diff characters
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several enhancements and robustness fixes to the Nova on-device AI assistant. Key changes include implementing a runaway/looped output detector (GenerationSafety), a unified tool call parser (ToolCallParser) to handle both JSON and ChatML-ish markup, and a user-facing "Stop" button to abort model generation mid-stream. It also restructures the adult mode policy prompt to place instructions at the beginning of the system prompt for better model adherence, and resolves UI crashes during streaming by disabling text selection on active markdown blocks. Feedback on these changes highlights opportunities to improve the repetition detector to support arbitrary period lengths, refine the markdown streaming UI to preserve rich formatting, and fix a potential bug in the tool call parser where replacing single quotes breaks queries containing apostrophes. Additionally, the markup stripper can be optimized to handle unclosed tags during active streaming.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| /// True when [text] ends with the same ~80-char block repeated ≥3 times. | ||
| static bool hasConsecutiveRepetition( | ||
| String text, { | ||
| int window = repetitionWindowChars, | ||
| int minRepeats = repetitionMinRepeats, | ||
| }) { | ||
| if (window < 16 || minRepeats < 2) return false; | ||
| final need = window * minRepeats; | ||
| if (text.length < need) return false; | ||
|
|
||
| final chunk = text.substring(text.length - window); | ||
| if (chunk.trim().isEmpty) return false; | ||
|
|
||
| var repeats = 1; | ||
| var end = text.length - window; | ||
| while (repeats < minRepeats && end >= window) { | ||
| final prev = text.substring(end - window, end); | ||
| if (prev != chunk) break; | ||
| repeats++; | ||
| end -= window; | ||
| } | ||
|
|
||
| return repeats >= minRepeats; | ||
| } |
There was a problem hiding this comment.
The current implementation of hasConsecutiveRepetition only detects repetitions where the period of the repeated substring is a divisor of window (80). If a model repeats a phrase of a different length (e.g., 6 characters like 'orange' or 35 characters), the shifted windows will not match, and the repetition will go undetected. To fix this, we should detect consecutive repetitions of any period p up to window.
static bool hasConsecutiveRepetition(
String text, {
int window = repetitionWindowChars,
int minRepeats = repetitionMinRepeats,
}) {
if (window < 16 || minRepeats < 2) return false;
for (int p = 1; p <= window; p++) {
if (text.length < p * minRepeats) continue;
final chunk = text.substring(text.length - p);
if (chunk.trim().isEmpty) continue;
var isRepeat = true;
for (int i = 1; i < minRepeats; i++) {
final prev = text.substring(text.length - p * (i + 1), text.length - p * i);
if (prev != chunk) {
isRepeat = false;
break;
}
}
if (isRepeat && p * minRepeats >= 15) {
return true;
}
}
return false;
}| static String stripMarkup(String text) { | ||
| var out = text; | ||
| out = out.replaceAll( | ||
| RegExp( | ||
| r'<\|?tool_call\|?>[\s\S]*?(?:</?\|?tool_call\|?>|<tool_call\|>)', | ||
| caseSensitive: false, | ||
| ), | ||
| '', | ||
| ); | ||
| out = out.replaceAll( | ||
| RegExp(r'call:\s*[a-zA-Z0-9_]+\s*\{[^{}]*\}', caseSensitive: false), | ||
| '', | ||
| ); |
There was a problem hiding this comment.
During streaming, the model outputs the tool call incrementally. Since the closing tag </|tool_call|> or } is not yet generated, the current regular expressions will not match, causing the raw tool call markup to temporarily flicker in the chat UI.
We can improve this by matching unclosed tags/blocks up to the end of the string ($).
| static String stripMarkup(String text) { | |
| var out = text; | |
| out = out.replaceAll( | |
| RegExp( | |
| r'<\|?tool_call\|?>[\s\S]*?(?:</?\|?tool_call\|?>|<tool_call\|>)', | |
| caseSensitive: false, | |
| ), | |
| '', | |
| ); | |
| out = out.replaceAll( | |
| RegExp(r'call:\s*[a-zA-Z0-9_]+\s*\{[^{}]*\}', caseSensitive: false), | |
| '', | |
| ); | |
| static String stripMarkup(String text) { | |
| var out = text; | |
| out = out.replaceAll( | |
| RegExp( | |
| r'<\|?tool_call\|?>[\s\S]*?(?:</?\|?tool_call\|?>|<tool_call\|>|$)', | |
| caseSensitive: false, | |
| ), | |
| '', | |
| ); | |
| out = out.replaceAll( | |
| RegExp(r'call:\s*[a-zA-Z0-9_]+\s*\{[^{}]*(?:\}|$)', caseSensitive: false), | |
| '', | |
| ); |
| // Selectable MarkdownBody during token streaming causes | ||
| // RangeError (selection past length) and | ||
| // `_dependents.isEmpty` InheritedWidget teardown crashes. | ||
| message.isStreaming | ||
| ? Text( | ||
| key: ValueKey('stream-text-${message.id}'), | ||
| message.text, | ||
| style: TextStyle( | ||
| color: isUser | ||
| ? Colors.white | ||
| : Colors.white.withValues(alpha: 0.92), | ||
| fontSize: 15, | ||
| height: 1.5, | ||
| ), | ||
| ) | ||
| : MarkdownBody( | ||
| key: ValueKey('md-${message.id}'), | ||
| data: message.text, | ||
| selectable: true, | ||
| styleSheet: MarkdownStyleSheet( | ||
| p: TextStyle( | ||
| color: isUser | ||
| ? Colors.white | ||
| : Colors.white.withValues(alpha: 0.92), | ||
| fontSize: 15, | ||
| height: 1.5, | ||
| ), | ||
| code: TextStyle( | ||
| backgroundColor: Colors.black26, | ||
| color: Colors.cyan[100], | ||
| fontFamily: 'monospace', | ||
| fontSize: 13, | ||
| ), | ||
| codeblockDecoration: BoxDecoration( | ||
| color: Colors.black38, | ||
| borderRadius: BorderRadius.circular(8), | ||
| ), | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Instead of falling back to a plain Text widget during streaming (which loses all markdown formatting, lists, and code blocks), we can use MarkdownBody with selectable: false. The RangeError crash is entirely caused by selectable: true (due to SelectableText selection offsets getting out of sync with rapid text updates). Using MarkdownBody(selectable: false) during streaming preserves rich formatting in real-time, and it will safely switch to selectable: true once streaming completes.
MarkdownBody(
key: ValueKey('md-${message.id}-${message.isStreaming}'),
data: message.text,
selectable: !message.isStreaming,
styleSheet: MarkdownStyleSheet(
p: TextStyle(
color: isUser
? Colors.white
: Colors.white.withValues(alpha: 0.92),
fontSize: 15,
height: 1.5,
),
code: TextStyle(
backgroundColor: Colors.black26,
color: Colors.cyan[100],
fontFamily: 'monospace',
fontSize: 13,
),
codeblockDecoration: BoxDecoration(
color: Colors.black38,
borderRadius: BorderRadius.circular(8),
),
),
),| final asJson = raw | ||
| .replaceAll(RegExp(r'<\|?"?\|>'), '"') | ||
| .replaceAll("'", '"'); |
There was a problem hiding this comment.
Using a global replaceAll("'", '"') to convert single quotes to double quotes will break any user query or argument that contains a legitimate apostrophe (e.g., "today's weather" or "what's the time"), making the JSON invalid and causing parsing to fail. Consider using a more robust parsing strategy or relying on the heuristic fallback directly for single-quoted strings.
|
Addressed Gemini review: broader repetition periods, strip incomplete tool markup while streaming, MarkdownBody(selectable:false) during stream, no apostrophe→quote rewrite in loose arg parse. |
Co-authored-by: Cursor <cursoragent@cursor.com>
Code Review SummaryStatus: No Issues Found | Recommendation: Merge All previously raised findings (repetition period breadth, streaming tool-markup flicker, Files Reviewed (13 files, substantive code)
Previous Review SummaryCurrent summary above is authoritative. Previous snapshots are kept for context only. Previous reviewStatus: No Issues Found | Recommendation: Merge All previously raised findings (repetition period breadth, streaming tool-markup flicker, Files Reviewed (13 files, substantive code)
Reviewed by step-3.7-flash · Input: 170.5K · Output: 28.4K · Cached: 2.8M |
Co-authored-by: Cursor <cursoragent@cursor.com>
1 participant
Summary
Test plan
Made with Cursor