Conversation
WalkthroughAdds spatial transcriptomics support to ClusterHierarchy by introducing a new field across three layers: a transient String field in the Java model with Jackson serialization, a test entry in the service layer, and a GraphQL schema field for external API exposure. Changes
✨ 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 |
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)
src/main/java/org/kpmp/cellTypeSummary/ClusterHierarchyService.java (1)
63-68: Potential NPE in comparator ifcellTypeOrderis null.The comparator calls
a.getCellTypeOrder().compareTo(b.getCellTypeOrder())directly. SincegetCellTypeOrder()returnsDouble(nullable, annotated with@Nullablein the model), any entry with a nullcellTypeOrderwill cause aNullPointerException.While the new entries (
bogusST,tiCluster) set this value explicitly, entries from the database viaclusterToHierarchycould potentially have null values.Suggested null-safe comparator
- Collections.sort(result, new Comparator<ClusterHierarchy>() { - `@Override` - public int compare(ClusterHierarchy a, ClusterHierarchy b) { - return a.getCellTypeOrder().compareTo(b.getCellTypeOrder()); - } - }); + Collections.sort(result, Comparator.comparing( + ClusterHierarchy::getCellTypeOrder, + Comparator.nullsLast(Comparator.naturalOrder()) + ));
🧹 Nitpick comments (1)
src/main/java/org/kpmp/cellTypeSummary/ClusterHierarchy.java (1)
14-14: Unused import.
JsonIncludeis imported but not used in this class. Consider removing it to keep imports clean.Suggested fix
-import com.fasterxml.jackson.annotation.JsonInclude;
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.