fix(frontend): fix MemoryCard rendering issues in Memories tab #1374
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughBroadened recommendation data shapes to accept objects or strings; rendering now extracts recommendation text from either shape, filters empty items, and requires content checks ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
Summary of ChangesHello @mateuszruszkowski, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses critical frontend rendering issues within the MemoryCard and PRReviewCard components. The primary goal is to eliminate 'black screen' errors and 'Objects are not valid as React child' exceptions that occurred when expanding content or displaying AI-generated responses. By refining data type definitions, adding robust content checks, and improving rendering logic for diverse data formats, the changes ensure a more stable and user-friendly display of information in the Memories tab. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
![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 addresses rendering issues in the MemoryCard and PRReviewCard components. The main fixes involve handling cases where parsed memory content is empty or contains objects instead of strings, preventing blank sections and React rendering errors. The changes correctly add fallbacks to display raw content when structured data is unavailable and update the rendering logic to handle new data shapes. My review includes a suggestion to improve the new rendering logic in MemoryCard.tsx for better maintainability and type safety.
AndyMik90
left a comment
AndyMik90
left a comment
There was a problem hiding this comment.
🤖 Auto Claude PR Review
Merge Verdict: 🟠 NEEDS REVISION
🟠 Needs revision - 4 issue(s) require attention.
Branch is out of date with base branch. Update branch first - if no conflicts arise, you can merge. If merge conflicts arise, resolve them and run follow-up review again. 4 issue(s) must be addressed (0 required, 4 recommended). 1 non-blocking suggestion(s) to consider.
Risk Assessment
| Factor | Level | Notes |
|---|---|---|
| Complexity | Low | Based on lines changed |
| Security Impact | None | Based on security findings |
| Scope Coherence | Good | Based on structural review |
🚨 Blocking Issues (Must Fix)
- Branch Out of Date: PR branch is behind the base branch and needs to be updated
Findings Summary
- Medium: 4 issue(s)
- Low: 1 issue(s)
Generated by Auto Claude PR Review
Findings (5 selected of 5 total)
🟡 [633f40874417] [MEDIUM] [Potential] Null element causes TypeError in recommendations_for_next_session mapping
📁 apps/frontend/src/renderer/components/context/MemoryCard.tsx:264
The type check typeof item === 'string' ? item : (item as { recommendation?: string }).recommendation fails when item is null because typeof null === 'object' in JavaScript. If the JSON data contains null in the array, accessing null.recommendation throws TypeError. The data source is JSON.parse() at line 49 which can contain null array elements.
Suggested fix:
Use optional chaining: `const text = typeof item === 'string' ? item : item?.recommendation;`
🟡 [20ffdad1fc32] [MEDIUM] [Potential] Null element causes TypeError in discoveries.recommendations mapping
📁 apps/frontend/src/renderer/components/context/MemoryCard.tsx:268
Same null vulnerability pattern as finding-1. When item is null, typeof null === 'object' evaluates true, so the code tries (null as { recommendation?: string }).recommendation which throws TypeError.
Suggested fix:
Use optional chaining: `const text = typeof item === 'string' ? item : item?.recommendation;`
🟡 [185e61c03997] [MEDIUM] [Potential] Null element causes TypeError in patterns_discovered mapping
📁 apps/frontend/src/renderer/components/context/MemoryCard.tsx:281
The pattern typeof pattern === 'string' ? pattern : pattern.pattern fails when pattern is null because typeof null === 'object' and null.pattern throws TypeError.
Suggested fix:
Use optional chaining: `const text = typeof pattern === 'string' ? pattern : pattern?.pattern;`
🟡 [dc8b7f7935ff] [MEDIUM] [Potential] Null element causes TypeError in gotchas_discovered mapping
📁 apps/frontend/src/renderer/components/context/MemoryCard.tsx:298
The pattern typeof gotcha === 'string' ? gotcha : gotcha.gotcha fails when gotcha is null because typeof null === 'object' and null.gotcha throws TypeError.
Suggested fix:
Use optional chaining: `const text = typeof gotcha === 'string' ? gotcha : gotcha?.gotcha;`
🔵 [083fee1129cd] [LOW] [Potential] Duplicated recommendation text extraction logic
📁 apps/frontend/src/renderer/components/context/MemoryCard.tsx:263
The exact same logic for extracting text from mixed-type recommendation arrays appears twice in adjacent code blocks. Both use typeof item === 'string' ? item : (item as { recommendation?: string }).recommendation. This violates DRY but is a minor concern since the duplication is localized.
Suggested fix:
Extract a helper function: `function getRecommendationText(item) { return typeof item === 'string' ? item : item?.recommendation; }`
This review was generated by Auto Claude.
mateuszruszkowski
force-pushed
the
fix/memorycard-rendering-v2
branch
from
2e066f4 to
bab8d12
Compare
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/context/MemoryCard.tsx (1)
361-366: Fallback block is unreachable.Same issue as in PRReviewCard: the expand button (line 179) only renders when
hasContentis true. Therefore,expandedcan never become true when!hasContent. The!parsedcase is also unreachable here becausehasContentreturns false when!parsed(line 115), which means no button is shown.The fallback at lines 113-128 for non-parseable content (when
!parsed) already handles the no-content case by not showing an expand button at all. Consider removing this unreachable block or adjusting the button visibility to allow raw JSON viewing.🔧 Suggested fix: Remove unreachable fallback
</div> )} - {/* Fallback for unparseable content or empty parsed content */} - {expanded && (!parsed || !hasContent) && ( - <pre className="mt-4 text-xs text-muted-foreground whitespace-pre-wrap font-mono p-3 bg-background rounded-lg max-h-64 overflow-auto border border-border/50"> - {memory.content} - </pre> - )} </CardContent>
🤖 Fix all issues with AI agents
In `@apps/frontend/src/renderer/components/context/PRReviewCard.tsx`:
- Around line 317-322: The JSX fallback rendering block that shows
memory.content when expanded && !hasExpandableContent is unreachable because the
expand button (which toggles expanded) is only rendered when
hasExpandableContent is true; remove the entire fallback fragment to avoid dead
code, and if you want to keep intent, replace it with a short comment in
PRReviewCard noting the previous fallback behavior; reference the expanded and
hasExpandableContent variables and the expand button logic (around the current
expand button rendering) when updating the component.
♻️ Duplicate comments (1)
apps/frontend/src/renderer/components/context/MemoryCard.tsx (1)
263-270: Clean implementation handling polymorphic recommendation items.The type narrowing via
typeof item === 'string'correctly handles both string and object recommendation formats without needing type assertions. This addresses the previous review feedback.
- Extract `getRecommendationText` helper function to eliminate duplicated logic for rendering recommendation items from both `recommendations_for_next_session` and `discoveries.recommendations` - Add `RecommendationItem` type alias for cleaner type definitions - Remove unreachable fallback code in PRReviewCard that could never be reached because the expand button only renders when hasExpandableContent is true Addresses CodeRabbit review comments in PR AndyMik90#1374. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
mateuszruszkowski
force-pushed
the
fix/memorycard-rendering-v2
branch
from
4ebdb87 to
489c859
Compare
AndyMik90
force-pushed
the
develop
branch
2 times, most recently
from
67a743f to
e83e445
Compare
Fixes two issues when expanding memory items:
1. Black screen when expanding memories with no displayable content
- Added hasContent/hasExpandableContent guards before rendering
- Show raw JSON fallback when parsed but no sections available
2. React error "Objects are not valid as a React child"
- Recommendations can be objects {category, recommendation, priority, effort}
- Now extracts text properly from both string and object formats
- Updated TypeScript interfaces to reflect actual data shape
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove unnecessary type assertions in recommendation mapping. TypeScript can infer the type from typeof narrowing. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove dead code block that could never execute: - The expand button only renders when hasExpandableContent is true - Therefore expanded && !hasExpandableContent is always false Addresses: coderabbitai review comment about unreachable code Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
mateuszruszkowski
force-pushed
the
fix/memorycard-rendering-v2
branch
from
489c859 to
d955e45
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/context/MemoryCard.tsx (1)
114-127:hasContentis missing a check fordiscoveries.recommendations.The
hasContentcheck doesn't includediscoveries.recommendations, but this array is rendered at lines 267-270. Ifdiscoveries.recommendationshas items but all other fields are empty,hasContentwill befalse, hiding the expand button and preventing users from seeing those recommendations.🐛 Proposed fix
const hasContent = useMemo(() => { if (!parsed) return false; const d = parsed.discoveries || {}; return ( (parsed.what_worked?.length ?? 0) > 0 || (parsed.what_failed?.length ?? 0) > 0 || (parsed.recommendations_for_next_session?.length ?? 0) > 0 || (d.patterns_discovered?.length ?? 0) > 0 || (d.gotchas_discovered?.length ?? 0) > 0 || (d.file_insights?.length ?? 0) > 0 || (d.changed_files?.length ?? 0) > 0 || + (d.recommendations?.length ?? 0) > 0 || d.approach_outcome?.approach_used ); }, [parsed]);
2 participants
Summary
Changes
apps/frontend/src/renderer/components/MemoryCard.tsx: Fix expanded content rendering with safe string conversionapps/frontend/src/renderer/components/PRReviewCard.tsx: Fix AI response rendering for non-string valuesTest plan
npm run typecheck🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.