Codex/monthly dump#112
Merged
Merged
Conversation
|
|
Contributor
|
Warning Review limit reached
More reviews will be available in 41 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughAdds a layout-switchable monthly photo grid builder (2x2/2x3) with service-side grid generation, gallery/entries pagination hooks, composed picker UI with camera capture, page integration validating month routes, AI skills docs, CI workflow, backend logging/test updates, and supporting tests/automation. ChangesAI Skills & Infrastructure
Monthly Dump Grid Creation Refactoring
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested Labels
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 12
🧹 Nitpick comments (3)
frontend/app/index.tsx (1)
13-17: ⚡ Quick winUse negative-first guard in the effect for guideline compliance.
Flip the condition to an early return guard (
if (!user || !session) return;) before callingprefetchSuggestedFriends().Suggested diff
useEffect(() => { - if (user && session) { - prefetchSuggestedFriends(); - } - }, [user, session, prefetchSuggestedFriends]) + if (!user || !session) return; + prefetchSuggestedFriends(); + }, [user, session, prefetchSuggestedFriends]);As per coding guidelines, "Always evaluate negative conditions first in conditional statements (e.g., check
!conditionand return early instead of checkingconditionand nesting logic)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/app/index.tsx` around lines 13 - 17, Change the effect to use a negative-first early-return guard: inside the useEffect callback check if !user || !session and return early, then call prefetchSuggestedFriends(); update the dependency array remains [user, session, prefetchSuggestedFriends]; locate the useEffect in frontend/app/index.tsx to modify the callback that currently tests if (user && session) before calling prefetchSuggestedFriends().frontend/components/monthly-dumps/monthly-dump-grid-icons.tsx (1)
4-47: 💤 Low value
mutedColoris declared but never used in either icon.
mutedColoris part ofGridIconPropsand destructured with a default in bothMonthlyDumpGrid2x2IconandMonthlyDumpGrid2x3Icon, but it is never referenced in the renderedRects. Either remove it or use it (e.g., to differentiate filled vs. empty cells).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/components/monthly-dumps/monthly-dump-grid-icons.tsx` around lines 4 - 47, mutedColor is declared in GridIconProps and destructured in MonthlyDumpGrid2x2Icon and MonthlyDumpGrid2x3Icon but never used; fix by applying mutedColor to the intended cells (or remove the prop if not needed). Specifically, update the GridCell calls inside MonthlyDumpGrid2x2Icon and MonthlyDumpGrid2x3Icon to pass color={mutedColor} for the cells that should appear muted (e.g., alternate or empty cells) while keeping color={color} for filled cells, referencing the GridCell component and the GridIconProps type; alternatively, if no muted styling is desired, remove mutedColor from GridIconProps and the destructured defaults in MonthlyDumpGrid2x2Icon and MonthlyDumpGrid2x3Icon.frontend/components/monthly-dumps/grid-image-picker-cell.tsx (1)
59-84: ⚡ Quick winInvert the
slotconditional to negative-first style.Please switch this ternary to
!slot ? emptyState : filledStateto match the repo rule.As per coding guidelines, "Always evaluate negative conditions first in conditional statements (e.g., check
!conditionand return early instead of checkingconditionand nesting logic)".🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/components/monthly-dumps/grid-image-picker-cell.tsx` around lines 59 - 84, The JSX currently branches with a positive-first ternary on `slot`; invert it to negative-first by checking `!slot` first and rendering the empty state (the <View style={styles.emptyCell}> block with `ImagePlus`) before the filled state to conform to the repo rule; keep all existing filled-state elements (the <Image source={{ uri: slot.content_url }} with `styles.gridImage`, the border `styles.gridCellBorder`, the focused action layer using `isFocused`/`pendingLayout`/`onRemove`/`Trash2`, and the removal overlay controlled by `pendingLayout && isRemovalSelected`) unchanged other than moving them into the else branch so behavior and handlers remain identical.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.agents/skills/frontend-design/SKILL.md:
- Line 4: The license filename reference in the SKILL metadata uses "license:
Complete terms in LICENSE.txt" which is inconsistent with the repo's actual
filename; update the value for the license key to reference the correct file
name (e.g., "license: Complete terms in LICENSE") so the 'license:' entry
matches the repository's LICENSE file and metadata consumers can resolve it
correctly.
In @.agents/skills/stop-slop/README.md:
- Line 13: The fenced code block in the README (the snippet showing the
stop-slop directory tree) lacks a language specifier and triggers MD040; update
the triple-backtick fence around that tree to include a language (e.g., change
``` to ```text) so the block is properly typed; edit the README.md entry
containing the directory tree (the fenced block showing "stop-slop/ ├── SKILL.md
...") and add the language token to the opening fence.
In @.github/workflows/frontend-tests.yml:
- Around line 18-21: Replace the floating action tags and disable credential
persistence: pin the actions/checkout and actions/setup-node uses to their
specific commit SHAs (replace actions/checkout@v5 and actions/setup-node@v4 with
the corresponding full commit SHA refs) and add the checkout input
persist-credentials: false (and optionally fetch-depth: 0) to the
actions/checkout step so credentials are not left in the runner; update the
workflow lines referencing actions/checkout and actions/setup-node accordingly.
In `@frontend/components/monthly-dumps/__tests__/photo-grid-picker.test.tsx`:
- Line 2: Remove the unused top-level import of Text, TouchableOpacity, and View
from 'react-native'—these symbols are only referenced via
require('react-native') inside the jest.mock factories (the mock blocks that
call require('react-native')), so delete the import line importing Text,
TouchableOpacity, View to satisfy the lint rule and keep the mock factories
unchanged.
In `@frontend/components/monthly-dumps/grid-image-picker-camera-modal.tsx`:
- Around line 54-56: The icon-only TouchableOpacity controls in
GridImagePickerCameraModal (the close button using onClose and
styles.cameraCloseButton and the capture button) lack accessibility props;
update both TouchableOpacity elements to include accessibilityLabel (e.g.,
"Close camera" and "Take photo"), accessibilityRole="button", and
accessibilityHint with concise action descriptions so screen readers announce
their purpose, ensuring you add these props to the TouchableOpacity that calls
onClose and to the capture handler TouchableOpacity.
In `@frontend/components/monthly-dumps/grid-image-picker-capture-canvas.tsx`:
- Around line 29-33: The ViewShot root can be collapsed on Android causing blank
PNG captures; update the ViewShot component (where viewShotRef is used and
styles.captureCanvas applied) to include collapsable={false} so the native view
isn't removed when off-screen, ensuring react-native-view-shot produces valid
captures.
In `@frontend/components/monthly-dumps/grid-image-picker-layout-popover.tsx`:
- Around line 75-82: The TouchableOpacity that renders the layout trigger
(testID "monthly-dump-grid-layout-button") and each icon-only layout option
should include accessibility properties so screen readers can discover them: add
accessibilityRole="button", a descriptive accessibilityLabel (e.g., "Grid layout
options" for the trigger or the specific layout name for each option), and an
accessibilityHint that explains the action (e.g., "Opens layout selection" or
"Selects the compact grid layout"); update the TouchableOpacity with
onPress={handleButtonPress} and the components that render CurrentLayoutIcon and
the option icons to include these
accessibilityRole/accessibilityLabel/accessibilityHint props so all interactive
icon-only controls are properly exposed to assistive tech.
In `@frontend/components/monthly-dumps/grid-image-picker.tsx`:
- Around line 188-196: Update the invalid 3-component rgba color in the
sheetContainer style: replace sheetContainer.backgroundColor = 'rgba(8,16,30)'
with a valid React Native color string such as 'rgb(8,16,30)' or
'rgba(8,16,30,1)' so the background applies correctly (look for the
sheetContainer style object used alongside SHEET_HEIGHT in
grid-image-picker.tsx).
In `@frontend/components/monthly-dumps/monthly-dump-grid-prompt-slide.tsx`:
- Line 4: Remove the unused Sparkles import from the lucide-react-native import
list (leave ArrowRight), and delete the now-unused style keys eyebrowRow,
eyebrowPill, eyebrowText, detailRow, detailChip, and detailChipText from the
styles object; ensure there are no remaining references to Sparkles or those
style keys elsewhere in this component and run lint to confirm no unused-symbol
errors remain.
In `@frontend/components/monthly-dumps/photo-grid-picker.tsx`:
- Line 446: The Text rendering in PhotoGridPicker currently always prints
"Remove {removeCount} photos" which is wrong for a single item; update the
string construction where styles.sourceTitle is used to pluralize based on
removeCount (e.g., in the component/function PhotoGridPicker where removeCount
is referenced) so it renders "Remove 1 photo" when removeCount === 1 and "Remove
N photos" otherwise (use a simple conditional or pluralization helper to choose
"photo" vs "photos").
- Around line 186-189: The tray "Entries" flow never clears focusedCellIndex, so
targetCellIndex = focusedCellIndex ?? emptyCellIndex always picks the stale
focused index and subsequent tray picks overwrite it; modify openSheet so it
resets the focus before showing the sheet (clear focusedCellIndex/nullify it) so
that targetCellIndex falls back to emptyCellIndex when assignPhotoToCell runs;
update openSheet (which currently checks isSubmitting and calls
setSheetVisible(true)) to call the focus-resetter (same state setter used by
handleCellPress) immediately before setSheetVisible(true).
In `@frontend/maestro/monthly-dumps/create-grid-from-gallery-flow.yaml`:
- Around line 12-35: The test flow taps the "open entries" button between tile
picks so GridImagePicker resets activeSource to 'entries' (useEffect on visible)
and subsequent picks (monthly-dump-grid-source-tile-1..5) will use Entries; to
fix, re-select the Gallery source before each tile selection (i.e., tap
"Gallery" prior to each monthly-dump-grid-source-tile-*), or alter the flow to
keep the sheet open across picks; look for GridImagePicker, its useEffect on
visible, and handleSelect which calls onClose when implementing the change.
---
Nitpick comments:
In `@frontend/app/index.tsx`:
- Around line 13-17: Change the effect to use a negative-first early-return
guard: inside the useEffect callback check if !user || !session and return
early, then call prefetchSuggestedFriends(); update the dependency array remains
[user, session, prefetchSuggestedFriends]; locate the useEffect in
frontend/app/index.tsx to modify the callback that currently tests if (user &&
session) before calling prefetchSuggestedFriends().
In `@frontend/components/monthly-dumps/grid-image-picker-cell.tsx`:
- Around line 59-84: The JSX currently branches with a positive-first ternary on
`slot`; invert it to negative-first by checking `!slot` first and rendering the
empty state (the <View style={styles.emptyCell}> block with `ImagePlus`) before
the filled state to conform to the repo rule; keep all existing filled-state
elements (the <Image source={{ uri: slot.content_url }} with `styles.gridImage`,
the border `styles.gridCellBorder`, the focused action layer using
`isFocused`/`pendingLayout`/`onRemove`/`Trash2`, and the removal overlay
controlled by `pendingLayout && isRemovalSelected`) unchanged other than moving
them into the else branch so behavior and handlers remain identical.
In `@frontend/components/monthly-dumps/monthly-dump-grid-icons.tsx`:
- Around line 4-47: mutedColor is declared in GridIconProps and destructured in
MonthlyDumpGrid2x2Icon and MonthlyDumpGrid2x3Icon but never used; fix by
applying mutedColor to the intended cells (or remove the prop if not needed).
Specifically, update the GridCell calls inside MonthlyDumpGrid2x2Icon and
MonthlyDumpGrid2x3Icon to pass color={mutedColor} for the cells that should
appear muted (e.g., alternate or empty cells) while keeping color={color} for
filled cells, referencing the GridCell component and the GridIconProps type;
alternatively, if no muted styling is desired, remove mutedColor from
GridIconProps and the destructured defaults in MonthlyDumpGrid2x2Icon and
MonthlyDumpGrid2x3Icon.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: b146b088-505e-4456-afc3-c1c12413c2f0
⛔ Files ignored due to path filters (2)
frontend/bun.lockis excluded by!**/*.lockfrontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (41)
.agents/skills/frontend-design/SKILL.md.agents/skills/stop-slop/CHANGELOG.md.agents/skills/stop-slop/LICENSE.agents/skills/stop-slop/README.md.agents/skills/stop-slop/SKILL.md.agents/skills/stop-slop/references/examples.md.agents/skills/stop-slop/references/phrases.md.agents/skills/stop-slop/references/structures.md.github/workflows/frontend-tests.ymlbackend/.gitignorebackend/services/notification_enqueue_service.pybackend/tests/test_monthly_dump_queue_notifications.pyfrontend/app/index.tsxfrontend/app/monthly-dumps/[month].tsxfrontend/components/monthly-dumps/__tests__/photo-grid-picker.test.tsxfrontend/components/monthly-dumps/grid-image-picker-bottom-tray.tsxfrontend/components/monthly-dumps/grid-image-picker-camera-modal.tsxfrontend/components/monthly-dumps/grid-image-picker-capture-canvas.tsxfrontend/components/monthly-dumps/grid-image-picker-cell.tsxfrontend/components/monthly-dumps/grid-image-picker-empty-state.tsxfrontend/components/monthly-dumps/grid-image-picker-layout-popover.tsxfrontend/components/monthly-dumps/grid-image-picker-right-actions.tsxfrontend/components/monthly-dumps/grid-image-picker-selection-pill.tsxfrontend/components/monthly-dumps/grid-image-picker.tsxfrontend/components/monthly-dumps/monthly-dump-banner.tsxfrontend/components/monthly-dumps/monthly-dump-grid-icons.tsxfrontend/components/monthly-dumps/monthly-dump-grid-prompt-slide.tsxfrontend/components/monthly-dumps/monthly-dump-image-slide.tsxfrontend/components/monthly-dumps/monthly-dump-progress-bar-item.tsxfrontend/components/monthly-dumps/monthly-dump-status-screen.tsxfrontend/components/monthly-dumps/photo-grid-empty-state.tsxfrontend/components/monthly-dumps/photo-grid-picker.tsxfrontend/hooks/use-gallery-images.tsfrontend/hooks/use-monthly-dump.tsfrontend/hooks/use-monthly-entries.tsfrontend/maestro/monthly-dumps/create-grid-from-entries-flow.yamlfrontend/maestro/monthly-dumps/create-grid-from-gallery-flow.yamlfrontend/package.jsonfrontend/services/__tests__/monthly-dump-service.test.tsfrontend/services/monthly-dump-service.tsskills-lock.json
| --- | ||
| name: frontend-design | ||
| description: Create distinctive, production-grade frontend interfaces with high design quality. Use this skill when the user asks to build web components, pages, or applications. Generates creative, polished code that avoids generic AI aesthetics. | ||
| license: Complete terms in LICENSE.txt |
Contributor
There was a problem hiding this comment.
Fix the license filename reference.
LICENSE.txt appears inconsistent with the repository naming convention used elsewhere (LICENSE). This can break metadata consumers and confuse readers.
Suggested fix
-license: Complete terms in LICENSE.txt
+license: Complete terms in LICENSE📝 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.
Suggested change
| license: Complete terms in LICENSE.txt | |
| license: Complete terms in LICENSE |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/frontend-design/SKILL.md at line 4, The license filename
reference in the SKILL metadata uses "license: Complete terms in LICENSE.txt"
which is inconsistent with the repo's actual filename; update the value for the
license key to reference the correct file name (e.g., "license: Complete terms
in LICENSE") so the 'license:' entry matches the repository's LICENSE file and
metadata consumers can resolve it correctly.
|
|
||
| ## Skill Structure | ||
|
|
||
| ``` |
Contributor
There was a problem hiding this comment.
Add a language to the fenced code block.
The code fence is missing a language specifier (MD040), which can fail markdown lint checks.
Suggested fix
-```
+```text
stop-slop/
├── SKILL.md # Core instructions
├── references/
│ ├── phrases.md # Phrases to remove
│ ├── structures.md # Structural patterns to avoid
│ └── examples.md # Before/after transformations
├── README.md
└── LICENSE</details>
<!-- suggestion_start -->
<details>
<summary>📝 Committable suggestion</summary>
> ‼️ **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.
```suggestion
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 13-13: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.agents/skills/stop-slop/README.md at line 13, The fenced code block in the
README (the snippet showing the stop-slop directory tree) lacks a language
specifier and triggers MD040; update the triple-backtick fence around that tree
to include a language (e.g., change ``` to ```text) so the block is properly
typed; edit the README.md entry containing the directory tree (the fenced block
showing "stop-slop/ ├── SKILL.md ...") and add the language token to the opening
fence.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation