[f] VER-261 - Optimize get snippets with labels#250
Conversation
WalkthroughUpdates label data model and components to use upvote_count/upvoted_by_me, adds optimistic updates and React Query cache invalidation. Adjusts snippet API pagination fields, halves PAGE_SIZE, introduces Vite env typings, and simplifies Supabase env usage. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant B as AddLabelButton
participant QC as QueryClient
participant API as Snippet API
participant S as Server
U->>B: Click "Add label"
B->>B: Build newLabel (uses upvote_count/upvoted_by_me)
B->>B: onLabelAdded(prev => [...prev, newLabel]) (optimistic)
B->>API: POST add label
API->>S: Create label
alt success
S-->>API: 200 OK
API-->>B: Success
B->>QC: invalidateQueries(['snippets','list', ...])
else error
S-->>API: Error
API-->>B: Error
B->>B: Revert optimistic label via updater
end
sequenceDiagram
autonumber
actor U as User
participant L as LabelButton
participant QC as QueryClient
participant API as Snippet API
participant S as Server
U->>L: Toggle upvote
L->>L: Optimistic isUpvoted/upvoteCount update
L->>API: PATCH toggle upvote
API->>S: Apply upvote change
alt success
S-->>API: 200 OK
API-->>L: Success
L->>QC: invalidateQueries(['snippets','list', ...])
else error
S-->>API: Error
API-->>L: Error
L->>L: Revert optimistic state
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 @quancao-ea, 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 introduces several optimizations aimed at improving the performance and maintainability of snippet and label management. Key changes include a more efficient data structure for labels, reducing the overhead associated with tracking upvotes, and integrating 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
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to 6f913ac in 1 minute and 26 seconds. Click for details.
- Reviewed
194lines of code in7files - Skipped
0files when reviewing. - Skipped posting
10draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/apis/snippet.ts:63
- Draft comment:
Removed explicit Number() conversion for total_pages and total_snippets. Ensure the API returns numbers to avoid type issues. - Reason this comment was not posted:
Comment looked like it was already resolved.
2. src/components/AddLabelButton.tsx:11
- Draft comment:
Updated onLabelAdded prop to allow functional updates. Verify that all consumers handle the new signature. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to verify that all consumers handle the new signature, which is against the rules. It doesn't provide a specific suggestion or ask for a specific test to be written.
3. src/components/AddLabelButton.tsx:37
- Draft comment:
Refactored newLabel object to use upvote_count and upvoted_by_me. Ensure backend and UI are updated for the new label structure. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that the backend and UI are updated for the new label structure. This falls under the category of asking the author to ensure something is tested or verified, which is against the rules.
4. src/components/AddLabelButton.tsx:60
- Draft comment:
Invalidates snippet queries after label creation. Confirm that the predicate correctly targets all necessary query keys. - Reason this comment was not posted:
Comment looked like it was already resolved.
5. src/components/LabelButton.tsx:23
- Draft comment:
Refactored upvote state initialization to use upvoted_by_me and upvote_count in place of previous upvoted_by array. Ensure consistency with new Label type. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is informative and suggests ensuring consistency with a new Label type, which is not a specific code suggestion or request for a test. It doesn't ask for confirmation of intention or suggest a specific change.
6. src/components/LabelButton.tsx:80
- Draft comment:
Invalidates snippet queries post upvote update. Verify the predicate matches all intended snippet list queries. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment asks the PR author to verify if the predicate matches all intended snippet list queries. This falls under asking the author to confirm or verify something, which is against the rules. Therefore, this comment should be removed.
7. src/constants/index.ts:6
- Draft comment:
PAGE_SIZE changed from 20 to 10. Confirm that this reduction in page size is intentional and its implications on pagination are acceptable. - Reason this comment was not posted:
Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% This comment directly violates one of the key rules: "Do NOT ask the PR author to confirm their intention". The comment is asking for confirmation and to check implications. We should assume the author knows what they're doing when making such a straightforward constant change. Maybe changing page size could have serious performance or UX implications that need to be discussed? While page size changes can have implications, asking for confirmation violates our rules. If there were specific, concrete issues, those should be raised instead. Delete this comment as it merely asks for confirmation of an intentional change without raising any specific issues.
8. src/lib/supabase.ts:3
- Draft comment:
Removed explicit string casting for VITE_SUPABASE_URL and VITE_SUPABASE_ANON_KEY. Ensure these environment variables are always defined to avoid runtime errors. - Reason this comment was not posted:
Comment looked like it was already resolved.
9. src/types/snippet.ts:24
- Draft comment:
Label interface has been refactored to include upvote_count and upvoted_by_me, removing several audit fields. Ensure all consumers are updated accordingly. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to ensure that all consumers are updated accordingly, which violates the rule against asking the author to ensure compatibility or review dependent parts. It does not provide a specific suggestion or ask for a specific test to be written.
10. src/vite-env.d.ts:1
- Draft comment:
vite-env.d.ts setup appears standard; no issues detected. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_NZQuaJZdafHuZ0Aj
You can customize 
![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 effectively optimizes snippet and label handling by simplifying the Label data structure, which reduces the data payload. The changes are well-implemented across the components, including updating AddLabelButton and LabelButton to use the new types and to correctly invalidate React Query caches, ensuring data consistency. The removal of unnecessary type conversions and the introduction of typed environment variables are also great improvements for code quality and robustness. Overall, this is a solid set of changes. I have one minor suggestion to improve the typing for a prop to make it more idiomatic.
| interface AddLabelButtonProps { | ||
| snippetId: string | ||
| onLabelAdded: (newLabels: Label[]) => void | ||
| onLabelAdded: (newLabels: Label[] | ((prevLabels: Label[]) => Label[])) => void |
There was a problem hiding this comment.
For better clarity and to follow React idioms, it's recommended to use the React.Dispatch<React.SetStateAction<T>> type for props that are state setters from useState. This makes the prop's purpose as a state dispatcher function immediately clear to other developers.
| onLabelAdded: (newLabels: Label[] | ((prevLabels: Label[]) => Label[])) => void | |
| onLabelAdded: React.Dispatch<React.SetStateAction<Label[]>> |
![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)
src/components/AddLabelButton.tsx (1)
74-81: Possible crash when allLabels is undefined.useLabels() may be loading; calling allLabels.filter will throw.
- const filteredSuggestions = allLabels + const source = allLabels ?? [] + const filteredSuggestions = source .filter(label => label.toLowerCase().includes(value.toLowerCase())) .slice(0, 10)
🧹 Nitpick comments (8)
src/apis/snippet.ts (1)
1-1: Fix header comment path.Update to reflect actual file path.
-// src/api/api.ts +// src/apis/snippet.tssrc/lib/supabase.ts (1)
3-4: Add runtime guards for missing env to fail fast.Typed envs can still be empty strings; throw a clear error early.
const supabaseUrl = import.meta.env.VITE_SUPABASE_URL const supabaseAnonKey = import.meta.env.VITE_SUPABASE_ANON_KEY + +if (!supabaseUrl || !supabaseAnonKey) { + const missing = [ + !supabaseUrl && 'VITE_SUPABASE_URL', + !supabaseAnonKey && 'VITE_SUPABASE_ANON_KEY' + ].filter(Boolean).join(', ') + throw new Error(`Missing required env: ${missing}`) +}Optionally wire Database types if available for stronger typing.
src/components/AddLabelButton.tsx (4)
36-42: Safer optimistic id generation.Prefer crypto.randomUUID() with Date.now() fallback to reduce collision risk.
- id: Date.now().toString(), + id: (typeof crypto !== 'undefined' && 'randomUUID' in crypto) + ? crypto.randomUUID() + : Date.now().toString(),
60-63: Use queryKey-based invalidation for stability.Predicate on indexes is brittle. Prefer queryKey form (v4/v5 support).
- queryClient.invalidateQueries({ - predicate: (query) => query.queryKey[0] === 'snippets' && query.queryKey[1] === 'list' - }) + queryClient.invalidateQueries({ queryKey: ['snippets', 'list'] })If on v3, keep predicate; if on v4/v5, the above is preferred.
88-91: Guard empty input and trim.Prevents blank/whitespace labels.
- if (e.key === 'Enter') { - createLabel(inputValue) + if (e.key === 'Enter') { + const value = inputValue.trim() + if (value) createLabel(value) }
112-119: Prefer stable keys over array index.Use suggestion as key since values are unique strings.
- {suggestions.map((suggestion, index) => ( + {suggestions.map((suggestion) => ( <li - key={index} + key={suggestion}src/components/LabelButton.tsx (2)
25-31: Harden defaults to avoid undefined state.Coalesce to booleans/numbers so UI never renders undefined and math never hits NaN.
Apply this diff:
- return localUpvoted !== null ? localUpvoted : label.upvoted_by_me + return localUpvoted !== null ? localUpvoted : !!label.upvoted_by_me @@ - return localCount !== null ? localCount : label.upvote_count + return localCount !== null ? localCount : (label.upvote_count ?? 0)
42-45: Optional: keep count in sync on logout and normalize boolean.Counts are global; consider always syncing
upvoteCount, and show not-upvoted when user logs out.Apply this diff:
- if (user) { - setIsUpvoted(label.upvoted_by_me) - setUpvoteCount(label.upvote_count) - } + setUpvoteCount(label.upvote_count ?? 0) + setIsUpvoted(user ? !!label.upvoted_by_me : false)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
src/apis/snippet.ts(1 hunks)src/components/AddLabelButton.tsx(4 hunks)src/components/LabelButton.tsx(4 hunks)src/constants/index.ts(1 hunks)src/lib/supabase.ts(1 hunks)src/types/snippet.ts(1 hunks)src/vite-env.d.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/components/AddLabelButton.tsx (1)
src/types/snippet.ts (1)
Label(24-29)
src/components/LabelButton.tsx (1)
src/lib/storage.ts (1)
getLocalStorageItem(1-9)
🔇 Additional comments (8)
src/vite-env.d.ts (1)
3-10: Vite env typings — LGTM.This unblocks safe import.meta.env usage across the app.
src/constants/index.ts (1)
6-6: PAGE_SIZE halved to 10 — confirm downstream assumptionsMatches fetchSnippets default (10). I couldn't verify in the sandbox (ripgrep skipped files); run these locally to find any remaining hardcoded 20s or PAGE_SIZE usages:
rg -n --hidden -S '\bPAGE_SIZE\b' || true rg -n --hidden -S 'pageSize\s*[:=]\s*20' || true rg -n --hidden -S '\b(limit|take|first)\s*[:=]\s*20' || true rg -n --hidden -S '20\s*(items|per\s*page|per_page|per-page)' || true # fallback if rg unavailable: git grep -n --heading --line-number -E "PAGE_SIZE|pageSize\s*[:=]\s*20|20\s*(items|per\s*page|per_page|per-page)" || truesrc/types/snippet.ts (1)
24-29: Label shape change — verified; no stale references found.
Occurrences only in src/types/snippet.ts, src/components/AddLabelButton.tsx, src/components/LabelButton.tsx.src/apis/snippet.ts (2)
27-33: Single source of truth for page sizeReplace the literal 10 with PAGE_SIZE from '@/constants'.
+import { PAGE_SIZE } from '@/constants' @@ -export const fetchSnippets = async ({ - pageParam = 0, - pageSize = 10, +export const fetchSnippets = async ({ + pageParam = 0, + pageSize = PAGE_SIZE,Also confirm page index (0 vs 1) matches the get_snippets RPC — frontend currently uses 0 (pageParam default 0; initialPageParam: 0).
62-67: Confirm RPC numeric types or coerce with Number()PaginatedResponse declares total_pages/total_snippets as number but supabase.rpc returns untyped data — if get_snippets can return numeric/decimal columns as strings, coerce before returning (Number(data.total_pages), Number(data.num_of_snippets)).
File: src/apis/snippet.ts (return block)
return { snippets: data.snippets, total_pages: data.total_pages, currentPage: pageParam, total_snippets: data.num_of_snippets }src/components/LabelButton.tsx (3)
6-6: Type import aligns with new Label shape.Good switch to the normalized fields (
upvoted_by_me,upvote_count).
10-10: React Query client usage looks correct.Import and hook usage are appropriate.
Also applies to: 21-21
80-83: Use queryKey-based invalidation for snippet listsThe predicate can miss queries with different key shapes; target the snippet lists explicitly.
File: src/components/LabelButton.tsx (invalidateQueries call)
Apply this diff:
- queryClient.invalidateQueries({ - predicate: (query) => query.queryKey[0] === 'snippets' && query.queryKey[1] === 'list' - }) + queryClient.invalidateQueries({ queryKey: ['snippets', 'list'] })Confirm all snippet-list queries use the same ['snippets','list'] key shape before merging.
| interface AddLabelButtonProps { | ||
| snippetId: string | ||
| onLabelAdded: (newLabels: Label[]) => void | ||
| onLabelAdded: (newLabels: Label[] | ((prevLabels: Label[]) => Label[])) => void | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Prop signature union may break existing call sites.
Parents typed as (labels: Label[]) => void won’t accept function args under strictFunctionTypes. Either (a) switch to always passing an updater: (updater: (prev: Label[]) => Label[]) => void, or (b) keep old signature and handle optimistic state locally.
🏁 Script executed:
#!/bin/bash
# Locate usages passing onLabelAdded to ensure they handle the union/function form
rg -nP -C2 '<AddLabelButton\b[^>]*onLabelAdded=' --type=tsxLength of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Checking for src/components/AddLabelButton.tsx..."
if [ -f src/components/AddLabelButton.tsx ]; then
echo "---- src/components/AddLabelButton.tsx (first 240 lines) ----"
sed -n '1,240p' src/components/AddLabelButton.tsx || true
else
echo "File not found: src/components/AddLabelButton.tsx"
fi
echo
echo "Searching repo for 'onLabelAdded' occurrences in ts/tsx/js/jsx files..."
rg -n --hidden --glob '!**/node_modules/**' "onLabelAdded" -g "**/*.{ts,tsx,js,jsx}" -S || true
echo
echo "Searching repo for JSX usage of AddLabelButton..."
rg -n --hidden --glob '!**/node_modules/**' "<AddLabelButton\\b" -g "**/*.{ts,tsx,js,jsx}" -S || true
echo
echo "Searching repo for identifier 'AddLabelButton'..."
rg -n --hidden --glob '!**/node_modules/**' "AddLabelButton" -g "**/*.{ts,tsx,js,jsx}" -S || trueLength of output: 6123
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- src/components/SnippetCard.tsx ----"
sed -n '1,400p' src/components/SnippetCard.tsx || true
echo
echo "---- src/components/SnippetDetail.tsx ----"
sed -n '1,800p' src/components/SnippetDetail.tsx || trueLength of output: 24597
onLabelAdded prop type is incompatible with SnippetCard under strictFunctionTypes
AddLabelButton defines onLabelAdded as (Label[] | ((prev: Label[]) => Label[])) => void, but src/components/SnippetCard.tsx provides handleLabelAdded: (newLabels: Label[]) => void — this is not assignable with strictFunctionTypes and will cause a TypeScript error. src/components/SnippetDetail.tsx already accepts the union.
- Fix (recommended): make the prop a SetStateAction dispatcher for clarity and compatibility — onLabelAdded: React.Dispatch<React.SetStateAction<Label[]>> — then pass setLabels (or typed handler) from parents (e.g., ).
- Alternate: update SnippetCard.handleLabelAdded signature to accept Label[] | ((prev: Label[]) => Label[]), or change AddLabelButton to only call updater functions and update types consistently.
Files: src/components/AddLabelButton.tsx, src/components/SnippetCard.tsx, src/components/SnippetDetail.tsx.
🤖 Prompt for AI Agents
In src/components/AddLabelButton.tsx around lines 9-12, the onLabelAdded prop is
typed as (newLabels: Label[] | ((prevLabels: Label[]) => Label[])) => void which
is incompatible with SnippetCard's (newLabels: Label[]) => void under
strictFunctionTypes; change the prop to a SetStateAction dispatcher:
onLabelAdded: React.Dispatch<React.SetStateAction<Label[]>> and update callers
to pass the state setter (e.g., setLabels) or an appropriately typed wrapper so
AddLabelButton can call it with either a Label[] or an updater function;
alternatively ensure SnippetCard and SnippetDetail use the same dispatcher type
if they currently pass setLabels.
quancao-ea
changed the title
2 participants
Important
Optimizes snippet and label handling, updates types and constants, and improves environment variable management.
fetchSnippetsinsnippet.tsby removing unnecessaryNumber()conversions.AddLabelButtonandLabelButtonto useupvote_countandupvoted_by_meinstead of detailed upvoter info.AddLabelButton, invalidates snippet queries after label creation.LabelButton, invalidates snippet queries after upvote toggle.Labelinterface insnippet.tsto includeupvote_countandupvoted_by_me.PAGE_SIZEinconstants/index.tsfrom 20 to 10.supabaseUrlandsupabaseAnonKeyinsupabase.ts.vite-env.d.tsfor Vite environment variable types.This description was created by
for 6f913ac. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Bug Fixes
Refactor
Chores