-
Notifications
You must be signed in to change notification settings - Fork 745
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: convert reddit pipe to next js #1055
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
nice!
|
i've added the the next cron time component and the test button |
@tribhuwan-kumar nice! i dont see the pipe.json file though ? also content should be {
"crons": [
{
"path": "/api/logpipeline",
"schedule": "0 */1 * * * *"
}
]
} since we now store the settings in the |
i'm writing a single config file which is pipe.json and its sync between app's settings (store.bin) the reddit {
"interval": 60,
"pageSize": 100000,
"summaryFrequency": "daily",
"emailAddress": "mail@com",
"emailPassword": "password",
"emailTime": "11:00",
"customPrompt": "daily",
"dailylogPrompt": "hello",
"contentType": "all",
"windowName": "windows",
"aiUrl": "https://models.inference.ai.azure.com/",
"aiModel": "gpt-4o-mini",
"openaiApiKey": "ghp_OILya30eZ****************IuUe8",
"crons": [
{
"path": "/api/logpipeline",
"schedule": "0 */1 * * * *"
}
]
} at frontend cron time is in seconds, so i'm just converting it minutes. |
sure, i'll do it in another pr
then we can simply remove the {
"crons": [
{
"path": "/api/pipeline",
"schedule": "0 */1 * * * *"
}
],
"pageSize": 100000,
"summaryFrequency": "daily",
"emailAddress": "",
"emailPassword": "",
"emailTime": "11:00",
"customPrompt": "dummy",
"dailylogPrompt": "daily",
"contentType": "all",
"windowName": "windows hello",
"aiUrl": "https://models.inference.ai.azure.com/",
"aiModel": "gpt-4o-mini",
"openaiApiKey": "ghp_***************************e8"
} now |
2025-01-06.06-14-33.mp4can you check this pipe https://github.com/tribhuwan-kumar/screenpipe/tree/main/pipes/reddit-auto-posts |
`; | ||
|
||
console.log("reddit questions prompt:", prompt); | ||
const openai = new OpenAI({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@louis030195
#1055 (comment)
can i get the screenpipe-cloud access?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure DM me your email you use to auth on screenpipe on discord or mail [email protected] i will give you free credit etc
@louis030195 |
its screenpipe's sdk to send app notification |
WalkthroughThe pull request introduces a comprehensive Next.js application for the Reddit auto-posts pipe. The project is structured as a modern web application with a focus on user interface, settings management, and automated Reddit post generation. It leverages React components, Tailwind CSS for styling, and integrates with various AI providers for content generation. The application provides a user-friendly interface for configuring Reddit post generation settings, with support for different AI models and email notifications. Changes
Assessment against linked issues
Poem
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 34
🧹 Nitpick comments (33)
pipes/reddit-auto-posts/components/ui/tooltip.tsx (1)
8-13
: LGTM! Clean component abstractions.Good use of component aliasing pattern. This provides a clean abstraction layer and allows for future customization if needed.
Consider adding JSDoc comments to document the purpose and usage of each component for better maintainability.
pipes/reddit-auto-posts/components/ui/badge.tsx (2)
26-28
: Consider using a more semantic HTML element.While
div
works, consider usingspan
for badges as they are typically inline elements. This would be more semantically correct.- extends React.HTMLAttributes<HTMLDivElement>, + extends React.HTMLAttributes<HTMLSpanElement>,
30-34
: Update element to match interface change.If adopting the previous suggestion to use
span
, ensure the rendered element matches:- <div className={cn(badgeVariants({ variant }), className)} {...props} /> + <span className={cn(badgeVariants({ variant }), className)} {...props} />pipes/reddit-auto-posts/components/ui/label.tsx (1)
1-26
: Consider adding documentation and testsTo improve maintainability and reliability, consider:
- Adding JSDoc documentation for the component and its props
- Including unit tests to verify accessibility features and rendering behavior
Example documentation:
/** * A form label component that supports all standard label attributes and styling variants. * Built on top of Radix UI's label primitive with added styling capabilities. * * @example * ```tsx * <Label htmlFor="email">Email address</Label> * <input id="email" type="email" /> * ``` */pipes/reddit-auto-posts/lib/hooks/use-sql-autocomplete.ts (1)
10-11
: Consider memory management for cacheThe current cache implementation could grow indefinitely as new types are added.
Consider implementing cache size limits and cleanup:
const MAX_CACHE_ITEMS = 10; const cache = new Map<string, { data: AutocompleteItem[]; timestamp: number }>(); // Add cleanup logic when setting cache if (cache.size >= MAX_CACHE_ITEMS) { const oldestKey = Array.from(cache.keys())[0]; cache.delete(oldestKey); }pipes/reddit-auto-posts/components/sql-autocomplete-input.tsx (2)
133-138
: Performance: Optimize filtering logicThe current filtering implementation could be inefficient for large datasets.
Consider memoizing the filtered items:
+ const filteredItems = useMemo(() => + items.filter((item) => + item.name.toLowerCase().includes(inputValue?.toLowerCase() ?? '') + ), + [items, inputValue] + ); - items - .filter((item) => - item.name.toLowerCase().includes(inputValue?.toLowerCase() as string) - ) + filteredItems
66-80
: Performance: Optimize click outside handlerThe click outside handler could be optimized using useCallback.
Memoize the handler function:
+ const handleClickOutside = useCallback((event: MouseEvent) => { - if (commandRef.current && !commandRef.current.contains(event.target as Node)) { - setOpen(false); - } + }, []); useEffect(() => { - const handleClickOutside = (event: MouseEvent) => { - if (commandRef.current && !commandRef.current.contains(event.target as Node)) { - setOpen(false); - } - }; document.addEventListener("mousedown", handleClickOutside); return () => { document.removeEventListener("mousedown", handleClickOutside); }; - }, []); + }, [handleClickOutside]);pipes/reddit-auto-posts/app/api/pipeline/route.ts (1)
112-112
: Simplify null checks using optional chaining.You can simplify the null checks on
screenData
andscreenData.data
by using optional chaining to make the code more concise.Apply this change to both instances:
- if (screenData && screenData.data && screenData.data.length > 0) { + if (screenData?.data?.length > 0) {Also applies to: 166-166
🧰 Tools
🪛 Biome (1.9.4)
[error] 112-112: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
pipes/reddit-auto-posts/lib/types.ts (1)
5-5
: Consider usingDate
type for 'timestamp' in 'DailyLog'.Using the
Date
type instead of a string fortimestamp
enhances type safety and facilitates date operations.Apply this change to update the type:
export interface DailyLog { activity: string; category: string; tags: string[]; - timestamp: string; + timestamp: Date; }Ensure that any code using
DailyLog
is updated accordingly to handle theDate
type.pipes/reddit-auto-posts/app/page.tsx (1)
7-7
: Consider using responsive design instead of fixed min-height.The hard-coded
min-h-[1250px]
could cause layout issues on different screen sizes. Consider using responsive classes or viewport units.- <main className="min-h-[1250px]"> + <main className="min-h-screen">pipes/reddit-auto-posts/components/header.tsx (1)
3-17
: Consider internationalizing text content.Extract text content to translation files for better maintainability and future internationalization support.
Example implementation using
next-intl
:import { useTranslations } from 'next-intl' export default function Header() { const t = useTranslations('Header') return ( <div className="flex flex-col justify-center items-center mt-8"> <Image className="w-24 h-24" src="/128x128.png" alt={t('logo.alt')} width={96} height={96} priority /> <h1 className="font-bold text-center text-2xl"> {t('title')} </h1> <h2 className='font-medium text-lg text-center mt-1'> {t('description')} </h2> </div> ) }pipes/reddit-auto-posts/app/layout.tsx (1)
6-9
: Consider enhancing metadata for better SEO.The implementation follows Next.js best practices. Consider adding more metadata fields like
keywords
,robots
, and OpenGraph tags for better SEO.export const metadata: Metadata = { title: "Reddit post pipe • Screenpipe", description: "Make reddit post using your Screenpipe data", + keywords: "reddit, automation, screenpipe, posts", + openGraph: { + title: "Reddit post pipe • Screenpipe", + description: "Make reddit post using your Screenpipe data", + type: "website" + } };pipes/reddit-auto-posts/components/ui/input.tsx (1)
5-19
: Enhance input component with validation and accessibility.The component implementation is solid. Consider adding:
- Built-in validation props
- ARIA attributes for better accessibility
const Input = React.forwardRef<HTMLInputElement, React.ComponentProps<"input">>( - ({ className, type, ...props }, ref) => { + ({ className, type, "aria-label": ariaLabel, "aria-describedby": ariaDescribedby, required, pattern, ...props }, ref) => { return ( <input type={type} + aria-label={ariaLabel} + aria-describedby={ariaDescribedby} + aria-required={required} className={cn( "flex h-10 w-full rounded-md border border-input bg-background px-3 py-2 text-base ring-offset-background file:border-0 file:bg-transparent file:text-sm file:font-medium file:text-foreground placeholder:text-muted-foreground focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-ring focus-visible:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 md:text-sm", className )} ref={ref} + required={required} + pattern={pattern} {...props} /> ) } )pipes/reddit-auto-posts/components/toaster.tsx (1)
13-35
: Add toast management features for better UX.Consider enhancing the Toaster component with:
- Maximum toast limit
- Auto-dismiss functionality
- Smooth animations
export function Toaster() { - const { toasts } = useToast() + const { toasts, dismissToast } = useToast() + const MAX_TOASTS = 3 + + React.useEffect(() => { + toasts.forEach(toast => { + if (toast.duration !== Infinity) { + const timer = setTimeout(() => dismissToast(toast.id), toast.duration || 5000) + return () => clearTimeout(timer) + } + }) + }, [toasts, dismissToast]) return ( <ToastProvider> - {toasts.map(function ({ id, title, description, action, ...props }) { + {toasts.slice(0, MAX_TOASTS).map(function ({ id, title, description, action, ...props }) { return ( <Toast key={id} {...props}> <div className="grid gap-1"> {title && <ToastTitle>{title}</ToastTitle>} {description && ( <ToastDescription>{description}</ToastDescription> )} </div> {action} <ToastClose /> </Toast> ) })} <ToastViewport /> </ToastProvider> ) }pipes/reddit-auto-posts/lib/actions/update-pipe-config.ts (1)
33-36
: Enhance error handling with specific error typesThe error handling is too generic. Consider adding specific error types and messages for different failure scenarios.
try { // ... existing code ... } catch (error) { if (error instanceof SyntaxError) { console.error("Invalid JSON format in pipe.json:", error); throw new Error("Configuration file is corrupted"); } if (error.code === 'ENOENT') { console.error("pipe.json not found:", error); throw new Error("Configuration file not found"); } console.error("Failed to save Reddit settings:", error); throw new Error("Failed to update configuration"); }pipes/reddit-auto-posts/lib/actions/generate-reddit-links.ts (2)
8-10
: Extract regex patterns as constantsThe regex patterns should be constants at the top of the file for better maintainability and reuse.
const TITLE_PATTERN = /\[TITLE\](.*?)\[\/TITLE\]/s; const BODY_PATTERN = /\[BODY\](.*?)\[\/BODY\]/s; const SUBREDDIT_PATTERN = /\[r\/.*?\]/g;
34-40
: Improve error handling for link generationThe try-catch block could be more specific about what errors it's catching and provide better error messages.
try { if (encodedTitle.length + encodedBody.length > 2000) { // Common URL length limit throw new Error("URL too long"); } const link = `https://www.reddit.com/r/${subredditName}/submit?title=${encodedTitle}&text=${encodedBody}`; result += `- ${subreddit} [SEND](${link})\n`; } catch (error) { const errorMessage = error instanceof Error ? error.message : "Unknown error"; console.warn(`Failed to generate link for post ${index + 1} in r/${subredditName}: ${errorMessage}`); result += `- ${subreddit} [POST TOO LONG - COPY MANUALLY]\n`; }pipes/reddit-auto-posts/components/ui/button.tsx (2)
7-34
: Enhance button accessibilityThe button variants should include ARIA attributes for better accessibility.
const buttonVariants = cva( // ... existing classes ... { variants: { variant: { default: "bg-primary text-primary-foreground hover:bg-primary/90 aria-pressed:bg-primary/80", destructive: "bg-destructive text-destructive-foreground hover:bg-destructive/90 aria-pressed:bg-destructive/80", // ... other variants }, // ... other properties } } )
42-53
: Add prop types documentationConsider adding JSDoc comments to document the component props for better developer experience.
/** * Button component that supports multiple variants and sizes * @param {Object} props - Component props * @param {string} [props.variant='default'] - Visual style variant * @param {string} [props.size='default'] - Size variant * @param {boolean} [props.asChild=false] - Whether to render as a child component * @param {string} [props.className] - Additional CSS classes */ const Button = React.forwardRef<HTMLButtonElement, ButtonProps>( // ... existing implementation )pipes/reddit-auto-posts/lib/actions/generate-log.ts (1)
58-63
: Improve JSON cleaning robustnessThe JSON cleaning logic could be more robust to handle various edge cases.
function cleanJsonResponse(content: string): string { return content .replace(/^[\s\n]*```(?:json)?/, '') // Remove starting code block .replace(/```[\s\n]*$/, '') // Remove ending code block .replace(/[\u0000-\u001F\u007F-\u009F]/g, '') // Remove control chars .trim(); } const cleanedResult = cleanJsonResponse(messageContent);pipes/reddit-auto-posts/app/api/settings/route.ts (1)
17-23
: Consider making paths configurableThe hard-coded path to
pipe.json
could be made more maintainable by moving it to a configuration file.- const screenpipeDir = process.env.SCREENPIPE_DIR || process.cwd(); - const settingsPath = path.join( - screenpipeDir, - "pipes", - "reddit-auto-posts", - "pipe.json" - ); + const settingsPath = path.join( + process.env.SCREENPIPE_DIR ?? process.env.CONFIG_DIR ?? process.cwd(), + "config", + "pipe.json" + );pipes/reddit-auto-posts/tailwind.config.ts (1)
7-11
: Consider optimizing content pathsThe content paths could be more specific to reduce build time.
content: [ - "./pages/**/*.{js,ts,jsx,tsx,mdx}", - "./components/**/*.{js,ts,jsx,tsx,mdx}", - "./app/**/*.{js,ts,jsx,tsx,mdx}", + "./pages/**/[!.]*{.js,.ts,.jsx,.tsx,.mdx}", + "./components/**/[!.]*{.js,.ts,.jsx,.tsx,.mdx}", + "./app/**/[!.]*{.js,.ts,.jsx,.tsx,.mdx}", ],pipes/reddit-auto-posts/lib/use-toast.ts (1)
132-141
: Consider using React Context instead of global listenersThe current implementation uses global state and listeners, which could cause memory leaks and make testing difficult.
Consider refactoring to use React Context for better state management and testability.
pipes/reddit-auto-posts/components/ui/toast.tsx (2)
18-21
: Consider adding screen reader announcementsWhile using Radix UI primitives provides good accessibility, explicit screen reader announcements could enhance the experience.
<ToastPrimitives.Viewport ref={ref} className={cn( "fixed top-0 z-[100] flex max-h-screen w-full flex-col-reverse p-4 sm:bottom-0 sm:right-0 sm:top-auto sm:flex-col md:max-w-[420px]", className )} + aria-live="polite" + role="status" {...props} />
115-117
: Add JSDoc comments for exported typesThe exported types lack documentation, which could make them harder to use correctly.
+/** Props for the Toast component including variant information */ type ToastProps = React.ComponentPropsWithoutRef<typeof Toast> +/** React element representing a toast action */ type ToastActionElement = React.ReactElement<typeof ToastAction>pipes/reddit-auto-posts/lib/hooks/use-settings.ts (2)
36-66
: Optimize settings refresh frequency.The current implementation refreshes settings every 30 seconds, which might be too frequent and could impact performance. Consider:
- Increasing the interval to a more reasonable duration (e.g., 5 minutes)
- Making the interval configurable
- Using a reactive approach where settings are only fetched when needed
68-103
: Enhance error handling with specific error messages.The error handling could be improved by:
- Adding specific error types and messages
- Logging error details for debugging
- Retrying failed requests
} catch (err) { + console.error('Failed to update setting:', { key, value, namespace }, err); + const errorMessage = err instanceof Error ? err.message : 'Unknown error occurred'; - setError(err as Error); + setError(new Error(`Failed to update setting: ${errorMessage}`)); }pipes/reddit-auto-posts/components/ui/select.tsx (1)
1-160
: Enhance accessibility features.Consider adding the following accessibility improvements:
- ARIA labels for better screen reader support
- Keyboard navigation instructions
- High contrast mode support
Example for SelectTrigger:
<SelectPrimitive.Trigger ref={ref} + aria-label="Open select menu" className={cn( "flex h-10 w-full items-center justify-between rounded-md border border-input bg-background px-3 py-2 text-sm ring-offset-background placeholder:text-muted-foreground focus:outline-none focus:ring-2 focus:ring-ring focus:ring-offset-2 disabled:cursor-not-allowed disabled:opacity-50 [&>span]:line-clamp-1", className )} {...props} >
pipes/reddit-auto-posts/components/pipe.tsx (1)
46-69
: Improve error handling intestPipe
.The error handling in the
testPipe
function could be improved:
- Add specific error messages for different failure cases
- Handle network errors separately
- Add retry logic for transient failures
pipes/reddit-auto-posts/components.json (1)
1-21
: Add documentation for configuration options.Consider adding comments or a separate documentation file explaining:
- The purpose of each configuration option
- How to customize the configuration
- The impact of each setting
pipes/reddit-auto-posts/README.md (2)
6-8
: Enhance feature description clarityThe current description is brief and could benefit from more detailed information about:
- How the activity monitoring works
- Types of questions it generates
- Examples of generated content
10-10
: Fix bare URL formattingConvert the bare URL to a proper markdown link with descriptive text.
-https://github.com/user-attachments/assets/289d1809-6855-4336-807f-dd9ee7181324 +[Watch Demo Video](https://github.com/user-attachments/assets/289d1809-6855-4336-807f-dd9ee7181324)🧰 Tools
🪛 Markdownlint (0.37.0)
10-10: null
Bare URL used(MD034, no-bare-urls)
pipes/reddit-auto-posts/app/globals.css (1)
27-31
: Document custom color variablesThe
--color-1
through--color-5
variables lack documentation explaining their specific use cases. Consider adding comments to clarify their purpose in the UI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (5)
pipes/reddit-auto-posts/app/favicon.ico
is excluded by!**/*.ico
pipes/reddit-auto-posts/bun.lockb
is excluded by!**/bun.lockb
pipes/reddit-auto-posts/package-lock.json
is excluded by!**/package-lock.json
pipes/reddit-auto-posts/public/128x128.png
is excluded by!**/*.png
screenpipe-app-tauri/src-tauri/gen/schemas/windows-schema.json
is excluded by!**/gen/**
📒 Files selected for processing (36)
pipes/reddit-auto-posts/.eslintrc.json
(1 hunks)pipes/reddit-auto-posts/.gitignore
(1 hunks)pipes/reddit-auto-posts/README.md
(1 hunks)pipes/reddit-auto-posts/app/api/pipeline/route.ts
(1 hunks)pipes/reddit-auto-posts/app/api/settings/route.ts
(1 hunks)pipes/reddit-auto-posts/app/globals.css
(1 hunks)pipes/reddit-auto-posts/app/layout.tsx
(1 hunks)pipes/reddit-auto-posts/app/page.tsx
(1 hunks)pipes/reddit-auto-posts/components.json
(1 hunks)pipes/reddit-auto-posts/components/header.tsx
(1 hunks)pipes/reddit-auto-posts/components/pipe.tsx
(1 hunks)pipes/reddit-auto-posts/components/spinner.tsx
(1 hunks)pipes/reddit-auto-posts/components/sql-autocomplete-input.tsx
(1 hunks)pipes/reddit-auto-posts/components/toaster.tsx
(1 hunks)pipes/reddit-auto-posts/components/ui/badge.tsx
(1 hunks)pipes/reddit-auto-posts/components/ui/button.tsx
(1 hunks)pipes/reddit-auto-posts/components/ui/input.tsx
(1 hunks)pipes/reddit-auto-posts/components/ui/label.tsx
(1 hunks)pipes/reddit-auto-posts/components/ui/select.tsx
(1 hunks)pipes/reddit-auto-posts/components/ui/toast.tsx
(1 hunks)pipes/reddit-auto-posts/components/ui/tooltip.tsx
(1 hunks)pipes/reddit-auto-posts/lib/actions/generate-log.ts
(1 hunks)pipes/reddit-auto-posts/lib/actions/generate-reddit-links.ts
(1 hunks)pipes/reddit-auto-posts/lib/actions/generate-reddit-question.ts
(1 hunks)pipes/reddit-auto-posts/lib/actions/send-email.ts
(1 hunks)pipes/reddit-auto-posts/lib/actions/update-pipe-config.ts
(1 hunks)pipes/reddit-auto-posts/lib/hooks/use-settings.ts
(1 hunks)pipes/reddit-auto-posts/lib/hooks/use-sql-autocomplete.ts
(1 hunks)pipes/reddit-auto-posts/lib/types.ts
(1 hunks)pipes/reddit-auto-posts/lib/use-toast.ts
(1 hunks)pipes/reddit-auto-posts/lib/utils.ts
(1 hunks)pipes/reddit-auto-posts/next.config.mjs
(1 hunks)pipes/reddit-auto-posts/package.json
(1 hunks)pipes/reddit-auto-posts/postcss.config.mjs
(1 hunks)pipes/reddit-auto-posts/tailwind.config.ts
(1 hunks)pipes/reddit-auto-posts/tsconfig.json
(1 hunks)
✅ Files skipped from review due to trivial changes (6)
- pipes/reddit-auto-posts/.eslintrc.json
- pipes/reddit-auto-posts/postcss.config.mjs
- pipes/reddit-auto-posts/next.config.mjs
- pipes/reddit-auto-posts/tsconfig.json
- pipes/reddit-auto-posts/.gitignore
- pipes/reddit-auto-posts/package.json
🧰 Additional context used
🪛 Markdownlint (0.37.0)
pipes/reddit-auto-posts/README.md
10-10: null
Bare URL used
(MD034, no-bare-urls)
🪛 Biome (1.9.4)
pipes/reddit-auto-posts/app/api/pipeline/route.ts
[error] 112-112: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 166-166: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: test-macos
- GitHub Check: test-windows
- GitHub Check: test-windows
- GitHub Check: test-linux
- GitHub Check: test-ubuntu
🔇 Additional comments (18)
pipes/reddit-auto-posts/components/ui/tooltip.tsx (3)
1-7
: LGTM! Proper setup of imports and client directive.The imports are well-organized and the "use client" directive is correctly placed at the top of the file.
30-30
: LGTM! Clean exports.The export pattern is clear and includes all necessary components.
14-28
: LGTM! Well-implemented TooltipContent component.The implementation follows React best practices with proper TypeScript types and ref forwarding. The styling includes comprehensive animations and positioning.
Verify that the z-index value of 50 is consistent with other overlay components in the application:
✅ Verification successful
Z-index value is consistent with other overlay components
The z-index value of 50 is consistently used across overlay components in the UI library, matching the implementation in the select dropdown component.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for z-index values in other UI components rg "z-\d+" pipes/reddit-auto-posts/components/ui/Length of output: 1009
pipes/reddit-auto-posts/components/ui/badge.tsx (3)
6-24
: Well-structured variant definitions with good accessibility!The badge variants are well-defined with:
- Clear visual hierarchy through different background colors
- Proper hover states for interaction feedback
- Focus rings for keyboard navigation
- Consistent spacing and typography
36-36
: Clean exports following best practices!Exporting both the component and variants is a good practice, allowing for reuse and consistent styling across the application.
1-4
: Verify the implementation of the utility function.The
cn
utility is imported from@/lib/utils
. Let's ensure it's properly implemented.✅ Verification successful
The
cn
utility is properly implemented.The utility function correctly combines
clsx
andtailwind-merge
for handling Tailwind CSS class merging, which is the expected implementation for the Badge component's styling needs.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the cn utility implementation rg -t typescript -t javascript "export.*function cn|export.*const cn"Length of output: 106
Script:
#!/bin/bash # Find potential utility files fd -e ts -e js utils.ts utils.js lib/utils.ts lib/utils.js # If files are found, display their contents fd -e ts -e js utils.ts utils.js lib/utils.ts lib/utils.js -x cat {}Length of output: 561
Script:
#!/bin/bash # Find utils files recursively fd utils -e ts -e js # Display contents of found files fd utils -e ts -e js -x cat {}Length of output: 20053
pipes/reddit-auto-posts/components/ui/label.tsx (4)
9-11
: Well-structured label variants with accessibility support!The label variants are well-defined with appropriate text styling and proper handling of disabled states through peer modifiers.
13-24
: Excellent implementation with proper TypeScript support and ref forwarding!The Label component is well-implemented with:
- Proper ref forwarding
- Strong TypeScript types
- Clean props handling
- Correct className merging
26-26
: Clean named export!The export statement follows the recommended pattern for React components.
7-7
: Verify the utils import path resolutionThe import uses an alias path
@/lib/utils
. Ensure that this path is properly configured in your TypeScript/Next.js configuration.✅ Verification successful
Import path is correctly configured
The
@/lib/utils
import is properly set up with:
- Path alias configured in tsconfig.json
- Utils file exists with the correct
cn
function export🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if the tsconfig.json or next.config.js contains the path alias configuration echo "Checking for path alias configuration..." rg -l '"@/lib"' tsconfig.json next.config.js # Verify the actual utils file exists fd -p 'lib/utils\.(ts|js|tsx|jsx)$'Length of output: 732
Script:
#!/bin/bash # Check for tsconfig.json in the pipe's directory fd tsconfig.json pipes/reddit-auto-posts # Verify the content of utils.ts to ensure it exports 'cn' cat pipes/reddit-auto-posts/lib/utils.tsLength of output: 289
Script:
#!/bin/bash # Check the path alias configuration in tsconfig.json cat pipes/reddit-auto-posts/tsconfig.json | grep -A 5 "paths"Length of output: 216
pipes/reddit-auto-posts/lib/actions/generate-reddit-question.ts (1)
34-41
: Avoid using 'dangerouslyAllowBrowser: true' in OpenAI configuration.Setting
dangerouslyAllowBrowser: true
can expose the API key in client-side environments, leading to security vulnerabilities. Since this function runs on the server, this option is unnecessary and should be removed.
[security_issue]Apply this diff to remove the insecure option:
const openai = new OpenAI({ apiKey: aiProviderType === "screenpipe-cloud" ? userToken : openaiApiKey, baseURL: gptApiUrl, - dangerouslyAllowBrowser: true, });
pipes/reddit-auto-posts/app/api/pipeline/route.ts (1)
56-57
: Ensure secure handling of email credentials.Storing email credentials in plaintext within settings poses a security risk. Use environment variables or a secure credential manager to protect sensitive information.
[security_issue]Adjust your configuration to retrieve credentials from environment variables:
- const emailAddress = redditSettings?.emailAddress; - const emailPassword = redditSettings?.emailPassword; + const emailAddress = process.env.EMAIL_ADDRESS; + const emailPassword = process.env.EMAIL_PASSWORD;Ensure that these environment variables are properly set and secured.
Also applies to: 87-88, 187-188
pipes/reddit-auto-posts/lib/utils.ts (1)
1-6
: LGTM! Well-implemented utility function.The
cn
utility function follows best practices for Tailwind CSS class merging:
- Proper TypeScript typing with
ClassValue[]
- Uses established libraries (
clsx
andtailwind-merge
)- Exports a reusable function for consistent class name handling
pipes/reddit-auto-posts/lib/actions/update-pipe-config.ts (1)
25-30
: Validate interval value before updating cron scheduleThe cron schedule is created without validating the interval value, which could lead to invalid schedules.
// Add validation before updating cron if (!Number.isInteger(redditSettings?.interval) || redditSettings.interval < 60) { throw new Error("Interval must be a positive integer >= 60 seconds"); } const cronMinutes = Math.floor(redditSettings.interval / 60); const schedule = `0 */${cronMinutes} * * * *`;✅ Verification successful
The review comment is correct - validation is necessary for cron schedule safety
The codebase shows the interval is used without validation, while evidence in
route.ts
confirms 60 seconds as the minimum expected value. The suggested validation would prevent invalid cron schedules and align with existing assumptions.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find type definitions and usages of redditSettings ast-grep --pattern 'interface $_ { interval: $_ }' # Look for any validation of interval rg "interval.*validation|validate.*interval" -B 2 -A 2 # Find other usages of interval in the context of redditSettings rg "redditSettings.*interval" -B 2 -A 2Length of output: 1041
Script:
#!/bin/bash # Look for schema or type definitions rg -t typescript -t javascript "redditSettings|interval" "schema.ts|types.ts|config.ts" -B 2 -A 2 # Check for any zod or other validation schemas ast-grep --pattern 'z.object({ $$$ interval: $_ $$$ })'Length of output: 198
pipes/reddit-auto-posts/lib/hooks/use-settings.ts (2)
105-143
: Enhance error handling with specific error messages.Similar to the
updateSetting
function, the error handling could be improved.
145-196
: Enhance error handling with specific error messages.Similar to the
updateSetting
function, the error handling could be improved.pipes/reddit-auto-posts/app/globals.css (2)
88-95
: LGTM! Well-structured base stylesThe base styles are well-organized and properly utilize Tailwind's apply directive for consistent theming.
40-44
: Verify chart color accessibilityPlease ensure that the chart colors meet WCAG 2.1 contrast requirements for data visualization accessibility. Consider using a color contrast checker tool to verify these values.
const response = await fetch("http://localhost:3030/raw_sql", { | ||
method: "POST", | ||
headers: { | ||
"Content-Type": "application/json", | ||
}, | ||
body: JSON.stringify({ query }), | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Avoid hardcoded localhost URL
The hardcoded localhost URL could cause issues in different environments and poses security risks.
Use environment variables for the API endpoint:
- const response = await fetch("http://localhost:3030/raw_sql", {
+ const response = await fetch(process.env.NEXT_PUBLIC_API_URL + "/raw_sql", {
📝 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.
const response = await fetch("http://localhost:3030/raw_sql", { | |
method: "POST", | |
headers: { | |
"Content-Type": "application/json", | |
}, | |
body: JSON.stringify({ query }), | |
}); | |
const response = await fetch(process.env.NEXT_PUBLIC_API_URL + "/raw_sql", { | |
method: "POST", | |
headers: { | |
"Content-Type": "application/json", | |
}, | |
body: JSON.stringify({ query }), | |
}); |
const query = ` | ||
SELECT ${ | ||
type === "app" ? "ocr.app_name" : "ocr.window_name" | ||
} as name, COUNT(*) as count | ||
FROM ocr_text ocr | ||
JOIN frames f ON ocr.frame_id = f.id | ||
WHERE f.timestamp > datetime('now', '-7 days') | ||
GROUP BY ${type === "app" ? "ocr.app_name" : "ocr.window_name"} | ||
ORDER BY count DESC | ||
LIMIT 100 | ||
`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Security: Protect against SQL injection
The SQL query construction using template literals could be vulnerable to SQL injection. While the type
parameter is constrained by TypeScript, it's better to use parameterized queries or validate the input.
Consider using a query builder or implementing proper SQL escaping:
- const query = `
- SELECT ${
- type === "app" ? "ocr.app_name" : "ocr.window_name"
- } as name, COUNT(*) as count
- FROM ocr_text ocr
- JOIN frames f ON ocr.frame_id = f.id
- WHERE f.timestamp > datetime('now', '-7 days')
- GROUP BY ${type === "app" ? "ocr.app_name" : "ocr.window_name"}
- ORDER BY count DESC
- LIMIT 100
- `;
+ const columnName = type === "app" ? "ocr.app_name" : "ocr.window_name";
+ const query = {
+ text: `
+ SELECT $1 as name, COUNT(*) as count
+ FROM ocr_text ocr
+ JOIN frames f ON ocr.frame_id = f.id
+ WHERE f.timestamp > datetime('now', '-7 days')
+ GROUP BY $1
+ ORDER BY count DESC
+ LIMIT 100
+ `,
+ values: [columnName]
+ };
Committable suggestion skipped: line range outside the PR's diff.
} catch (error) { | ||
console.error("failed to fetch items:", error); | ||
} finally { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance error handling with user feedback
The error is only logged to console, which doesn't help users understand what went wrong.
Consider adding error state and message:
+ const [error, setError] = useState<string | null>(null);
try {
// ... existing code
} catch (error) {
console.error("failed to fetch items:", error);
+ setError(error instanceof Error ? error.message : "Failed to fetch items");
}
Committable suggestion skipped: line range outside the PR's diff.
<div className="relative"> | ||
<Input | ||
ref={inputRef} | ||
id={id} | ||
type="text" | ||
name={name} | ||
placeholder={placeholder} | ||
value={inputValue} | ||
defaultValue={defaultValue} | ||
onChange={handleInputChange} | ||
onFocus={() => setOpen(true)} | ||
className={cn("pr-8 w-full", icon ? "pl-7" : "pl-3")} | ||
autoCorrect="off" | ||
aria-autocomplete="none" | ||
onKeyDown={onKeyDown} | ||
/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Enhance accessibility attributes
The input component is missing important accessibility attributes.
Add ARIA attributes for better screen reader support:
<Input
ref={inputRef}
id={id}
type="text"
name={name}
placeholder={placeholder}
value={inputValue}
defaultValue={defaultValue}
onChange={handleInputChange}
onFocus={() => setOpen(true)}
className={cn("pr-8 w-full", icon ? "pl-7" : "pl-3")}
autoCorrect="off"
aria-autocomplete="none"
+ aria-expanded={open}
+ aria-controls={`${id}-listbox`}
+ aria-haspopup="listbox"
+ role="combobox"
onKeyDown={onKeyDown}
/>
📝 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.
<div className="relative"> | |
<Input | |
ref={inputRef} | |
id={id} | |
type="text" | |
name={name} | |
placeholder={placeholder} | |
value={inputValue} | |
defaultValue={defaultValue} | |
onChange={handleInputChange} | |
onFocus={() => setOpen(true)} | |
className={cn("pr-8 w-full", icon ? "pl-7" : "pl-3")} | |
autoCorrect="off" | |
aria-autocomplete="none" | |
onKeyDown={onKeyDown} | |
/> | |
<div className="relative"> | |
<Input | |
ref={inputRef} | |
id={id} | |
type="text" | |
name={name} | |
placeholder={placeholder} | |
value={inputValue} | |
defaultValue={defaultValue} | |
onChange={handleInputChange} | |
onFocus={() => setOpen(true)} | |
className={cn("pr-8 w-full", icon ? "pl-7" : "pl-3")} | |
autoCorrect="off" | |
aria-autocomplete="none" | |
aria-expanded={open} | |
aria-controls={`${id}-listbox`} | |
aria-haspopup="listbox" | |
role="combobox" | |
onKeyDown={onKeyDown} | |
/> |
.map((item: any) => ( | ||
<Command.Item | ||
key={item.name} | ||
value={item.name} | ||
onSelect={handleSelect} | ||
className="px-4 py-2 hover:bg-gray-100 cursor-pointer border-b border-gray-200 last:border-b-0" | ||
> | ||
{item.name} ({item.count}) | ||
</Command.Item> | ||
)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Type safety: Avoid using 'any' type
The item mapping uses 'any' type which defeats TypeScript's type checking.
Use proper typing:
- .map((item: any) => (
+ .map((item: AutocompleteItem) => (
Committable suggestion skipped: line range outside the PR's diff.
const handleSave = async (e: React.FormEvent) => { | ||
e.preventDefault(); | ||
const formData = new FormData(e.target as HTMLFormElement); | ||
|
||
const newRedditSettings = { | ||
interval: parseInt(formData.get("interval") as string), | ||
pageSize: parseInt(formData.get("pageSize") as string), | ||
summaryFrequency: formData.get("summaryFrequency") as string, | ||
emailAddress: formData.get("emailAddress") as string, | ||
emailPassword: formData.get("emailPassword") as string, | ||
emailTime: (formData).get("emailTime") as string, | ||
customPrompt: formData.get("customPrompt") as string, | ||
dailylogPrompt: formData.get("dailylogPrompt") as string, | ||
windowName: formData.get("windowName") as string, | ||
contentType: contentType as string, | ||
lastIntervalChangeTime: new Date().toISOString() | ||
} | ||
|
||
try { | ||
await updateSettings(newRedditSettings, "reddit-auto-posts"); | ||
await updatePipeConfig(newRedditSettings); | ||
toast({ | ||
title: "settings saved", | ||
description: "your reddit pipe settings have been updated", | ||
}); | ||
} catch (err) { | ||
toast({ | ||
variant: "destructive", | ||
title: "error", | ||
description: "failed to save settings", | ||
}); | ||
} | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add form validation.
The form submission lacks proper validation:
- Validate email format
- Ensure required fields are filled
- Validate numeric inputs
const { toast } = useToast(); | ||
const [showKey, setShowKey] = useState(false); | ||
const [loading, setLoading] = useState(false); | ||
const [lastLog, setLastLog] = useState<any>(null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Avoid using any
type.
Replace any
with a proper type definition for the log data structure.
-const [lastLog, setLastLog] = useState<any>(null);
+interface Log {
+ // Define the log structure here
+ timestamp: string;
+ message: string;
+ // Add other relevant fields
+}
+const [lastLog, setLastLog] = useState<Log | null>(null);
Committable suggestion skipped: line range outside the PR's diff.
const getNextCronTime = (lastIntervalChangeTime: string, interval: number) => { | ||
const lastChangeDate = new Date(lastIntervalChangeTime) || new Date().toISOString(); | ||
const now = new Date(); | ||
let nextCronTime = new Date(lastChangeDate.getTime()); | ||
nextCronTime.setMinutes(nextCronTime.getMinutes() + interval); | ||
while (nextCronTime < now) { | ||
nextCronTime.setMinutes(nextCronTime.getMinutes() + interval); | ||
} | ||
return nextCronTime.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix potential timezone issues in getNextCronTime
.
The function might have timezone-related issues:
- Use UTC dates for consistency
- Handle DST transitions
- Consider user's timezone
-const lastChangeDate = new Date(lastIntervalChangeTime) || new Date().toISOString();
+const lastChangeDate = lastIntervalChangeTime ? new Date(lastIntervalChangeTime) : new Date();
+const userTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone;
-return nextCronTime.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' });
+return nextCronTime.toLocaleTimeString([], {
+ hour: '2-digit',
+ minute: '2-digit',
+ timeZone: userTimezone
+});
📝 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.
const getNextCronTime = (lastIntervalChangeTime: string, interval: number) => { | |
const lastChangeDate = new Date(lastIntervalChangeTime) || new Date().toISOString(); | |
const now = new Date(); | |
let nextCronTime = new Date(lastChangeDate.getTime()); | |
nextCronTime.setMinutes(nextCronTime.getMinutes() + interval); | |
while (nextCronTime < now) { | |
nextCronTime.setMinutes(nextCronTime.getMinutes() + interval); | |
} | |
return nextCronTime.toLocaleTimeString([], { hour: '2-digit', minute: '2-digit' }); | |
}; | |
const getNextCronTime = (lastIntervalChangeTime: string, interval: number) => { | |
const lastChangeDate = lastIntervalChangeTime ? new Date(lastIntervalChangeTime) : new Date(); | |
const userTimezone = Intl.DateTimeFormat().resolvedOptions().timeZone; | |
const now = new Date(); | |
let nextCronTime = new Date(lastChangeDate.getTime()); | |
nextCronTime.setMinutes(nextCronTime.getMinutes() + interval); | |
while (nextCronTime < now) { | |
nextCronTime.setMinutes(nextCronTime.getMinutes() + interval); | |
} | |
return nextCronTime.toLocaleTimeString([], { | |
hour: '2-digit', | |
minute: '2-digit', | |
timeZone: userTimezone | |
}); | |
}; |
const isMacOS = () => { | ||
return navigator.platform.toUpperCase().indexOf('MAC') >= 0; | ||
}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replace platform detection with feature detection.
Using navigator.platform
is deprecated. Use a more reliable method for platform detection.
-const isMacOS = () => {
- return navigator.platform.toUpperCase().indexOf('MAC') >= 0;
-};
+const isMacOS = () => {
+ return navigator.userAgent.toLowerCase().includes('mac');
+};
Committable suggestion skipped: line range outside the PR's diff.
#### quick setup | ||
1. [Get an OpenAI API key](https://platform.openai.com/account/api-keys) | ||
2. [Create an app-specific password](https://support.google.com/accounts/answer/185833?hl=en) in your Google account that will be used to send yourself emails | ||
3. Configure pipe in the app UI, save, enable, and restart screenpipe recording (you can configure to either receive an email daily or several every x hours) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Add more detailed configuration instructions
The configuration step needs more specific details about:
- Required settings
- Available AI models
- Email frequency options
- Expected behavior after setup
/closes #997
/claim #997
the ui is same as the obsidian pipe and make sure to sync the setting across the pipe and app. it'll use the default ai settings as set to the app! i'll add a demo video after doing some posts. its functionality is same as prior
description
brief description of the changes in this pr.
related issue: #
how to test
add a few steps to test the pr in the most time efficient way.
if relevant add screenshots or screen captures to prove that this PR works to save us time.
if you think it can take more than 30 mins for us to review, we will pay between $20 and $200 for someone to review it for us, just ask in the pr.
Summary by CodeRabbit
New Features
User Interface
Technical Improvements