fix: make AI summary JSON parsing more robust#69
Conversation
📝 WalkthroughWalkthroughThis change improves the AI response parsing robustness in Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/services/aiService.ts (1)
354-358: Code fence stripping only matches at exact boundaries.The regex anchors (
^and$) only strip code fences when they're at the very start/end of the trimmed content. If the AI returns something likeHere's the JSON:\n\``json\n{...}\n````, the fences won't be stripped.This is acceptable since
extractAndParseAIJsonwill still locate the JSON via brace-matching, but a more permissive pattern could handle additional edge cases:🔧 Optional: More permissive code fence stripping
const cleaned = content .trim() - .replace(/^```(?:json)?\s*/i, '') - .replace(/\s*```$/i, '') + .replace(/```(?:json)?\s*/gi, '') .trim();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/services/aiService.ts` around lines 354 - 358, The current code that constructs the cleaned string only strips triple-backtick fences when they are at the absolute start/end (the const cleaned block); change the regexes to remove any triple-backtick fences (optionally followed/preceded by "json") anywhere in content rather than using ^/$ anchors (e.g., replace occurrences of /```(?:json)?\s*/gi and /```/gi or a single global pattern) so cleaned will have fences removed even when they are not the exact boundaries; update the replacement logic inside the const cleaned definition in aiService.ts accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/services/aiService.ts`:
- Around line 370-375: The current return uses cleaned.substring(0, 50) which
yields an empty string when cleaned is empty; update the summary fallback so
that if cleaned is falsy or trimmed length is 0 it returns a clear message like
"Unable to generate summary" (or "Analysis failed" depending on context) instead
of an empty string—modify the return logic where cleaned is used (the block that
builds { summary: ..., tags: [], platforms: [] }) to check cleaned.trim().length
and choose the explicit fallback message, otherwise keep the existing
substring(0, 50) + ellipsis behavior.
---
Nitpick comments:
In `@src/services/aiService.ts`:
- Around line 354-358: The current code that constructs the cleaned string only
strips triple-backtick fences when they are at the absolute start/end (the const
cleaned block); change the regexes to remove any triple-backtick fences
(optionally followed/preceded by "json") anywhere in content rather than using
^/$ anchors (e.g., replace occurrences of /```(?:json)?\s*/gi and /```/gi or a
single global pattern) so cleaned will have fences removed even when they are
not the exact boundaries; update the replacement logic inside the const cleaned
definition in aiService.ts accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
|
|
||
| return { | ||
| summary: content.substring(0, 50) + '...', | ||
| summary: cleaned.substring(0, 50) + (cleaned.length > 50 ? '...' : ''), | ||
| tags: [], | ||
| platforms: [], | ||
| }; |
There was a problem hiding this comment.
Empty content fallback yields an empty summary string.
If cleaned is empty (e.g., the AI returned only code fences with no content), cleaned.substring(0, 50) returns "". This is inconsistent with the other fallback paths that return explicit messages like "Unable to generate summary" (line 365) or "Analysis failed" (line 379).
🔧 Proposed fix for consistency
return {
- summary: cleaned.substring(0, 50) + (cleaned.length > 50 ? '...' : ''),
+ summary: cleaned.trim()
+ ? cleaned.substring(0, 50) + (cleaned.length > 50 ? '...' : '')
+ : (this.language === 'zh' ? '无法生成概述' : 'Unable to generate summary'),
tags: [],
platforms: [],
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return { | |
| summary: content.substring(0, 50) + '...', | |
| summary: cleaned.substring(0, 50) + (cleaned.length > 50 ? '...' : ''), | |
| tags: [], | |
| platforms: [], | |
| }; | |
| return { | |
| summary: cleaned.trim() | |
| ? cleaned.substring(0, 50) + (cleaned.length > 50 ? '...' : '') | |
| : (this.language === 'zh' ? '无法生成概述' : 'Unable to generate summary'), | |
| tags: [], | |
| platforms: [], | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/services/aiService.ts` around lines 370 - 375, The current return uses
cleaned.substring(0, 50) which yields an empty string when cleaned is empty;
update the summary fallback so that if cleaned is falsy or trimmed length is 0
it returns a clear message like "Unable to generate summary" (or "Analysis
failed" depending on context) instead of an empty string—modify the return logic
where cleaned is used (the block that builds { summary: ..., tags: [],
platforms: [] }) to check cleaned.trim().length and choose the explicit fallback
message, otherwise keep the existing substring(0, 50) + ellipsis behavior.
1 participant
Summary
Why
Issue #67 reports garbled AI summaries. The core problem is not display encoding, but brittle parsing when the model returns:
The previous implementation used a greedy regex to extract
{ ... }, which fails easily when the response is incomplete or wrapped in markdown.Validation
npm run buildsrc/services/aiService.tsCloses #67
Summary by CodeRabbit
Release Notes