Skip to content
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

Merged
merged 39 commits into from
Jan 14, 2025
Merged

Conversation

tribhuwan-kumar
Copy link
Contributor

@tribhuwan-kumar tribhuwan-kumar commented Dec 28, 2024


/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


image

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

    • Introduced a Reddit auto-posts pipe with functionality to generate and post questions based on screen data.
    • Implemented settings management for configuring Reddit post generation.
    • Created an email notification system for daily logs and Reddit questions.
    • Developed AI-powered content generation using OpenAI's GPT models.
    • Added a code block component for syntax highlighting and download functionality.
    • Introduced a SQL autocomplete input component for improved user experience.
    • Added a new layout component for consistent page structure.
    • Introduced a spinner component for loading states.
    • Enhanced the toast notification system for user feedback.
    • Added various new components including Header, Pipe, and Toaster for better user interaction.
    • Introduced a tooltip component for additional context.
  • User Interface

    • Designed a responsive web interface for configuring Reddit auto-posts.
    • Added toast notifications and tooltips for user feedback.
    • Implemented custom input components with autocomplete and validation.
    • Introduced various UI components such as badges, buttons, and icons for enhanced user experience.
    • Integrated new icons for improved visual representation.
  • Technical Improvements

    • Set up a Next.js application with TypeScript.
    • Configured Tailwind CSS for styling.
    • Integrated Radix UI components.
    • Added ESLint for code quality.
    • Implemented a custom hook for managing clipboard actions.
    • Established a structured logging mechanism for daily activities.
    • Introduced configuration files for improved project management and dependency handling.
    • Added a comprehensive .gitignore file to maintain a clean repository.

Copy link

vercel bot commented Dec 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
screenpipe ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 14, 2025 11:33pm

@louis030195
Copy link
Collaborator

nice!

  • can you use the cron feature of pipes.json like in the obsidian one instead of the infinite loop? lmk if you have any issue with it and i can fix the runtime, but it should work, use it with obsidian for days
  • can you make sure to use this-syntax for the naming of the folder eg reddit-auto-post
  • can you add a test button below save? it's useful otherwise people get confused and don't know if its working or not, allow them to test it out
  • ideally you can just write a timer that display the next time it will run somehow, that would be awesome!

@tribhuwan-kumar
Copy link
Contributor Author

  • can you use the cron feature of pipes.json like in the obsidian one instead of the infinite loop? lmk if you have any issue with it and i can fix the runtime, but it should work, use it with obsidian for days
{
  "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 * * * *"
    }
  ]
}

i've added the cron instead of relying on infinite loop

image

the next cron time component

image

and the test button

@louis030195
Copy link
Collaborator

@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 store.bin shared with tauri

@tribhuwan-kumar
Copy link
Contributor Author

tribhuwan-kumar commented Dec 30, 2024

@louis030195

i'm writing a single config file which is pipe.json and its sync between app's settings (store.bin)
https://github.com/tribhuwan-kumar/screenpipe/blob/5bb36f136ab716dbb430f7ad7c6c7045642fec50/pipes/reddit-auto-posts/lib/actions/update-pipe-config.ts#L17
but in obisidian pipe there are two json file one is settings.json and another is pipe.json

the reddit pipe.json content would be something like this:

{
  "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.
should i also write two json files for this pipe?

@louis030195
Copy link
Collaborator

i see, claude hallucinated and i did not check ...

it should be only pipe.json no settings.json

any chance you can fix the obsidian one too? like just remove settings.json and use the pipe.json

@tribhuwan-kumar

this is my pipe.json in obsidian:

Screenshot 2024-12-30 at 8 35 30 AM

we should not write interval to the pipe.json, we should just have the cron thing and if front end need to read it we can just use a cron js lib parsing thing

@tribhuwan-kumar
Copy link
Contributor Author

tribhuwan-kumar commented Dec 30, 2024

any chance you can fix the obsidian one too? like just remove settings.json and use the pipe.json

sure, i'll do it in another pr

we should not write interval to the pipe.json, we should just have the cron thing and if front end need to read it we can just use a cron js lib parsing thing

then we can simply remove the interval writing to pipe.json

{
  "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 pipe.json would be something like this

@louis030195
Copy link
Collaborator

Screenshot 2025-01-10 at 4 26 30 PM

i run screenpipe 24/7

@louis030195
Copy link
Collaborator

Screenshot 2025-01-10 at 4 37 10 PM

this is wrong!!!!!

it should show the list of questions and not the log

@tribhuwan-kumar
Copy link
Contributor Author

tribhuwan-kumar commented Jan 11, 2025

Screenshot 2025-01-10 at 4 37 10 PM

this is wrong!!!!!

it should show the list of questions and not the log

image

did you mean this?

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (3)
pipes/reddit-auto-posts/app/api/pipeline/route.ts (1)

125-131: Extract screen data querying logic to reduce duplication.

The screen data querying logic is duplicated in multiple places.

Apply this diff to extract the common logic:

+async function queryScreenData(params: {
+  startTime: string;
+  endTime: string;
+  windowName: string;
+  pageSize: number;
+  contentType: string;
+}) {
+  return retry(() => pipe.queryScreenpipe(params));
+}

-    const screenData = await retry(() => pipe.queryScreenpipe({
+    const screenData = await queryScreenData({
       startTime: startTime.toISOString(),
       endTime: now.toISOString(),
       windowName: windowName,
       limit: pageSize,
       contentType: contentType,
-    }));
+    });

Also applies to: 180-186

pipes/reddit-auto-posts/components/markdown.tsx (1)

4-9: Consider all props in memo comparison function.

The current implementation only compares children and className, ignoring other props from the Options type. This could lead to unnecessary re-renders if other props change.

 export const MemoizedReactMarkdown: FC<Options> = memo(
   ReactMarkdown,
-  (prevProps, nextProps) =>
-    prevProps.children === nextProps.children &&
-    prevProps.className === nextProps.className
+  (prevProps, nextProps) => {
+    const relevantProps = ['children', 'className', 'remarkPlugins', 'components'] as const;
+    return relevantProps.every(prop => prevProps[prop] === nextProps[prop]);
+  }
 )
pipes/reddit-auto-posts/components/pipe.tsx (1)

291-320: Improve markdown rendering container.

The markdown container has a fixed width that might cause layout issues on different screen sizes.

-            className="prose break-words dark:prose-invert prose-p:leading-relaxed prose-pre:p-0 w-[35vw] text-sm"
+            className="prose break-words dark:prose-invert prose-p:leading-relaxed prose-pre:p-0 max-w-full text-sm"
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 12d313c and 8fea9e9.

⛔ Files ignored due to path filters (1)
  • pipes/reddit-auto-posts/bun.lockb is excluded by !**/bun.lockb
📒 Files selected for processing (4)
  • pipes/reddit-auto-posts/app/api/pipeline/route.ts (1 hunks)
  • pipes/reddit-auto-posts/components/markdown.tsx (1 hunks)
  • pipes/reddit-auto-posts/components/pipe.tsx (1 hunks)
  • pipes/reddit-auto-posts/package.json (1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
pipes/reddit-auto-posts/app/api/pipeline/route.ts

[error] 134-134: Change to an optional chain.

Unsafe fix: Change to an optional chain.

(lint/complexity/useOptionalChain)


[error] 188-188: 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-windows
  • GitHub Check: test-macos
  • GitHub Check: test-linux
  • GitHub Check: test-ubuntu
  • GitHub Check: test-windows
🔇 Additional comments (10)
pipes/reddit-auto-posts/app/api/pipeline/route.ts (5)

1-10: LGTM! Well-organized imports following Next.js conventions.

The imports are properly structured with a clear separation between built-in Node.js modules and custom application modules.


158-177: ⚠️ Potential issue

Persist 'lastEmailSent' to accurately track email schedules.

The lastEmailSent variable resets on each function call, causing the email schedule logic to malfunction.

Apply this diff to persist the email schedule:

+async function getLastEmailSent(): Promise<Date> {
+  const screenpipeDir = process.env.SCREENPIPE_DIR || process.cwd();
+  const stateFile = path.join(screenpipeDir, "pipes", "reddit-auto-posts", "state.json");
+  try {
+    const state = JSON.parse(await fs.promises.readFile(stateFile, 'utf-8'));
+    return new Date(state.lastEmailSent);
+  } catch {
+    return new Date(0);
+  }
+}
+
+async function updateLastEmailSent(date: Date): Promise<void> {
+  const screenpipeDir = process.env.SCREENPIPE_DIR || process.cwd();
+  const stateFile = path.join(screenpipeDir, "pipes", "reddit-auto-posts", "state.json");
+  await fs.promises.writeFile(
+    stateFile,
+    JSON.stringify({ lastEmailSent: date.toISOString() })
+  );
+}

-    let lastEmailSent = new Date(0);
+    const lastEmailSent = await getLastEmailSent();

Likely invalid or redundant comment.


270-276: 🛠️ Refactor suggestion

Improve error handling security and consistency.

The current error handling:

  1. Exposes internal error details
  2. Uses inconsistent status codes
  3. Lacks proper error categorization

Apply this diff to improve error handling:

+const ERROR_MESSAGES = {
+  INTERNAL_ERROR: 'An internal error occurred',
+  INVALID_REQUEST: 'Invalid request parameters',
+  UNAUTHORIZED: 'Unauthorized access',
+  NOT_FOUND: 'Resource not found',
+} as const;
+
+function sanitizeError(error: unknown): { message: string; status: number } {
+  console.error('Detailed error:', error);
+  
+  if (error instanceof Error) {
+    if (error.message.includes('access')) {
+      return { message: ERROR_MESSAGES.UNAUTHORIZED, status: 401 };
+    }
+    if (error.message.includes('not found')) {
+      return { message: ERROR_MESSAGES.NOT_FOUND, status: 404 };
+    }
+    if (error.message.includes('invalid')) {
+      return { message: ERROR_MESSAGES.INVALID_REQUEST, status: 400 };
+    }
+  }
+  return { message: ERROR_MESSAGES.INTERNAL_ERROR, status: 500 };
+}

   } catch (error) {
     console.error("error in GET handler:", error);
+    const { message, status } = sanitizeError(error);
     return NextResponse.json(
-      { error: `${error}` },
-      { status: 400 }
+      { error: message },
+      { status }
     );
   }

Likely invalid or redundant comment.


11-31: 🛠️ Refactor suggestion

Improve log file handling for better reliability and performance.

The current implementation has several areas for improvement:

  1. Uses synchronous file operations which can block the event loop
  2. Lacks file size limits
  3. Basic error handling could be enhanced

Apply this diff to improve the implementation:

 async function saveDailyLog(logEntry: DailyLog) {
   if (!logEntry){
     throw new Error("no log entry to save")
   }
   console.log("saving log entry:", logEntry);

   const screenpipeDir = process.env.SCREENPIPE_DIR || process.cwd();
   const logsDir = path.join(screenpipeDir, "pipes", "reddit-auto-posts", "logs");
   const timestamp = new Date()
     .toISOString()
     .replace(/:/g, "-")
     .replace(/\..+/, "");
   const filename = `${timestamp}-${logEntry.category?.replace(/[\/\\?%*:|"<>']/g, "-")}.json`;
   const logFile = path.join(logsDir, filename)
+  const logContent = JSON.stringify(logEntry, null, 2);
+
+  // Add size limit (e.g., 1MB)
+  if (Buffer.byteLength(logContent) > 1024 * 1024) {
+    throw new Error("Log entry too large");
+  }
+
   try {
-    fs.writeFileSync(logFile, JSON.stringify(logEntry, null, 2));
+    await fs.promises.writeFile(logFile, logContent);
   } catch (error) {
     console.log(`Failed to write log file: ${error}`)
     throw new Error(`failed to write log file: ${error}`)
   }
 }

Likely invalid or redundant comment.


48-82: 🛠️ Refactor suggestion

Add proper settings validation to prevent runtime errors.

The settings retrieval lacks proper validation and relies on non-null assertions.

Apply this diff to add proper validation:

+interface RedditSettings {
+  interval: number;
+  summaryFrequency: string;
+  emailTime: string;
+  emailAddress?: string;
+  emailPassword?: string;
+  customPrompt: string;
+  dailylogPrompt: string;
+  windowName?: string;
+  pageSize: number;
+  contentType: string;
+}

+function validateSettings(settings: unknown): settings is RedditSettings {
+  if (!settings || typeof settings !== 'object') return false;
+  const s = settings as Record<string, unknown>;
+  return (
+    typeof s.interval === 'number' &&
+    typeof s.summaryFrequency === 'string' &&
+    typeof s.emailTime === 'string' &&
+    typeof s.customPrompt === 'string' &&
+    typeof s.dailylogPrompt === 'string' &&
+    typeof s.pageSize === 'number'
+  );
+}

 export async function GET() {
   try {
     const settingsManager = pipe.settings;
     const redditSettings = await pipe.settings.getNamespaceSettings("reddit-auto-posts");
+    if (!validateSettings(redditSettings)) {
+      return NextResponse.json(
+        { error: "Invalid settings configuration" },
+        { status: 400 }
+      );
+    }

Likely invalid or redundant comment.

pipes/reddit-auto-posts/package.json (1)

16-17: 🛠️ Refactor suggestion

Specify exact versions for internal packages.

Internal packages (@screenpipe/js and @screenpipe/browser) should use exact versions to ensure consistency.

-    "@screenpipe/js": "1.0.0",
-    "@screenpipe/browser": "0.1.12",
+    "@screenpipe/js": "1.0.0",
+    "@screenpipe/browser": "0.1.12"

Likely invalid or redundant comment.

pipes/reddit-auto-posts/components/pipe.tsx (4)

29-29: Replace any type with proper interface.

Using any type reduces type safety. Define a proper interface for the log structure.


78-87: Fix timezone handling in getNextCronTime.

The function has potential timezone issues that could affect scheduling accuracy.


89-91: Replace deprecated platform detection.

Using navigator.platform is deprecated and should be replaced with a more reliable method.


93-125: Add form validation in handleSave.

The form submission lacks proper validation for required fields and input formats.

Comment on lines +33 to +46
async function retry(fn: any, retries = 3, delay = 5000) {
for (let i = 0; i < retries; i++) {
try {
const result = await fn();
if (result){
return result;
}
} catch (error) {
console.log(`Screenpipe query failed, retry, attempt: ${i + 1}`)
if (i === retries - 1) throw error;
await new Promise(res => setTimeout(res, delay));
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add proper type safety to the retry function.

The function uses any type which bypasses TypeScript's type checking benefits.

Apply this diff to improve type safety:

-async function retry(fn: any, retries = 3, delay = 5000) {
+async function retry<T>(
+  fn: () => Promise<T | undefined>,
+  retries = 3,
+  delay = 5000
+): Promise<T | undefined> {
   for (let i = 0; i < retries; i++) {
     try {
       const result = await fn();
       if (result){
         return result;
       }
     } catch (error) {
       console.log(`Screenpipe query failed, retry, attempt: ${i + 1}`)
       if (i === retries - 1) throw error;
       await new Promise(res => setTimeout(res, delay));
     }
   }
 }
📝 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
async function retry(fn: any, retries = 3, delay = 5000) {
for (let i = 0; i < retries; i++) {
try {
const result = await fn();
if (result){
return result;
}
} catch (error) {
console.log(`Screenpipe query failed, retry, attempt: ${i + 1}`)
if (i === retries - 1) throw error;
await new Promise(res => setTimeout(res, delay));
}
}
}
async function retry<T>(
fn: () => Promise<T | undefined>,
retries = 3,
delay = 5000
): Promise<T | undefined> {
for (let i = 0; i < retries; i++) {
try {
const result = await fn();
if (result){
return result;
}
} catch (error) {
console.log(`Screenpipe query failed, retry, attempt: ${i + 1}`)
if (i === retries - 1) throw error;
await new Promise(res => setTimeout(res, delay));
}
}
}

Comment on lines +36 to +46
"devDependencies": {
"@tailwindcss/typography": "^0.5.13",
"@types/react-syntax-highlighter": "^15.5.13",
"@types/node": "^20",
"@types/react": "^18",
"@types/react-dom": "^18",
"eslint": "^8",
"eslint-config-next": "14.2.4",
"postcss": "^8",
"tailwindcss": "^3.4.1",
"typescript": "^5"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add missing peer dependencies.

The project uses Tailwind CSS but is missing some common peer dependencies.

   "devDependencies": {
     "@tailwindcss/typography": "^0.5.13",
     "@types/react-syntax-highlighter": "^15.5.13",
     "@types/node": "^20",
     "@types/react": "^18",
     "@types/react-dom": "^18",
     "eslint": "^8",
     "eslint-config-next": "14.2.4",
     "postcss": "^8",
     "tailwindcss": "^3.4.1",
-    "typescript": "^5"
+    "typescript": "^5",
+    "autoprefixer": "^10.0.0",
+    "@types/lodash": "^4.14.0"
📝 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
"devDependencies": {
"@tailwindcss/typography": "^0.5.13",
"@types/react-syntax-highlighter": "^15.5.13",
"@types/node": "^20",
"@types/react": "^18",
"@types/react-dom": "^18",
"eslint": "^8",
"eslint-config-next": "14.2.4",
"postcss": "^8",
"tailwindcss": "^3.4.1",
"typescript": "^5"
"devDependencies": {
"@tailwindcss/typography": "^0.5.13",
"@types/react-syntax-highlighter": "^15.5.13",
"@types/node": "^20",
"@types/react": "^18",
"@types/react-dom": "^18",
"eslint": "^8",
"eslint-config-next": "14.2.4",
"postcss": "^8",
"tailwindcss": "^3.4.1",
"typescript": "^5",
"autoprefixer": "^10.0.0",
"@types/lodash": "^4.14.0"
}

"cmdk": "0.2.1",
"lodash": "^4.17.21",
"lucide-react": "^0.468.0",
"next": "14.2.15",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

Next.js version 14.2.15 does not exist

The specified version "14.2.15" is invalid and doesn't exist in the npm registry. The latest stable version is 15.1.4, and the latest v14 versions are canary releases of 14.3.0. Please update to a valid Next.js version.

🔗 Analysis chain

Verify Next.js version compatibility.

The specified Next.js version (14.2.15) seems unusual as it's beyond the latest stable version. This might be a typo.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check latest Next.js version
curl -s https://registry.npmjs.org/next/latest | jq -r '.version'

Length of output: 72


Script:

#!/bin/bash
# Check if version 14.2.15 exists and list recent v14 versions
curl -s https://registry.npmjs.org/next | jq -r '.versions | keys[]' | grep "^14\." | sort -V | tail -n 5

Length of output: 192

Comment on lines +134 to +142
<Input
id="interval"
name="interval"
type="number"
defaultValue={settings.customSettings?.["reddit-auto-posts"]?.interval || 60}
placeholder="value in seconds"
className="flex-1"
/>
</div>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add input validation for interval.

The interval input needs minimum and maximum value constraints to prevent invalid configurations.

 <Input
   id="interval"
   name="interval"
   type="number"
+  min="30"
+  max="3600"
   defaultValue={settings.customSettings?.["reddit-auto-posts"]?.interval || 60}
   placeholder="value in seconds"
   className="flex-1"
+  required
 />
📝 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
<Input
id="interval"
name="interval"
type="number"
defaultValue={settings.customSettings?.["reddit-auto-posts"]?.interval || 60}
placeholder="value in seconds"
className="flex-1"
/>
</div>
<Input
id="interval"
name="interval"
type="number"
min="30"
max="3600"
defaultValue={settings.customSettings?.["reddit-auto-posts"]?.interval || 60}
placeholder="value in seconds"
className="flex-1"
required
/>
</div>

Comment on lines +49 to +76
const testPipe = async () => {
setLoading(true);
try {
const res = await fetch("/api/pipeline");
if (res.status === 500 || res.status === 400) {
toast({
title: "failed to intialize daily log",
description: "please check your credentials",
variant: "destructive"
})
} else if (res.status === 200) {
toast({
title: "pipe initalized sucessfully",
variant: "default"
})
}
const data = await res.json();
if (data.suggestedQuestions) {
setLastLog(data.suggestedQuestions);
} else {
setLastLog(JSON.stringify(data, null, 2));
}
} catch (err) {
console.error("error testing log:", err);
} finally {
setLoading(false);
}
};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Improve error handling in testPipe.

The function has several issues:

  1. Attempts to parse JSON response even for error cases
  2. Generic error message for all failure cases
  3. Missing loading state reset on early returns
 const testPipe = async () => {
   setLoading(true);
   try {
     const res = await fetch("/api/pipeline");
+    const data = await res.json();
     if (res.status === 500 || res.status === 400) {
+      setLoading(false);
       toast({
         title: "failed to intialize daily log",
-        description: "please check your credentials",
+        description: data.error || "please check your credentials",
         variant: "destructive"
       }) 
+      return;
     } else if (res.status === 200) {
       toast({
         title: "pipe initalized sucessfully",
         variant: "default"
       }) 
-    }
-    const data = await res.json();
     if (data.suggestedQuestions) {
       setLastLog(data.suggestedQuestions);
     } else {
       setLastLog(JSON.stringify(data, null, 2));
     }
+    }
   } catch (err) {
     console.error("error testing log:", err);
+    toast({
+      title: "Error",
+      description: "Failed to test pipe. Please try again.",
+      variant: "destructive"
+    });
   } finally {
     setLoading(false);
   }
 };
📝 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
const testPipe = async () => {
setLoading(true);
try {
const res = await fetch("/api/pipeline");
if (res.status === 500 || res.status === 400) {
toast({
title: "failed to intialize daily log",
description: "please check your credentials",
variant: "destructive"
})
} else if (res.status === 200) {
toast({
title: "pipe initalized sucessfully",
variant: "default"
})
}
const data = await res.json();
if (data.suggestedQuestions) {
setLastLog(data.suggestedQuestions);
} else {
setLastLog(JSON.stringify(data, null, 2));
}
} catch (err) {
console.error("error testing log:", err);
} finally {
setLoading(false);
}
};
const testPipe = async () => {
setLoading(true);
try {
const res = await fetch("/api/pipeline");
const data = await res.json();
if (res.status === 500 || res.status === 400) {
setLoading(false);
toast({
title: "failed to intialize daily log",
description: data.error || "please check your credentials",
variant: "destructive"
})
return;
} else if (res.status === 200) {
toast({
title: "pipe initalized sucessfully",
variant: "default"
})
if (data.suggestedQuestions) {
setLastLog(data.suggestedQuestions);
} else {
setLastLog(JSON.stringify(data, null, 2));
}
}
} catch (err) {
console.error("error testing log:", err);
toast({
title: "Error",
description: "Failed to test pipe. Please try again.",
variant: "destructive"
});
} finally {
setLoading(false);
}
};

@louis030195
Copy link
Collaborator

Screenshot 2025-01-10 at 4 37 10 PM this is wrong!!!!! it should show the list of questions and not the log

image

did you mean this?

yes

@louis030195
Copy link
Collaborator

ideally there should be a page to see history of all posts it created

https://ui.shadcn.com/docs/components/data-table

but could be next step

@louis030195
Copy link
Collaborator

Screenshot 2025-01-11 at 9 49 24 AM still get this

@louis030195
Copy link
Collaborator

btw the questions are quite good i was impressed - even though i use kinda poor model (gemini-1.5-flash-8b)

@louis030195
Copy link
Collaborator

how can i get the markdown question at the bottom? if there is no logs yet it can just generate a log and markdown or anything that makes generate the question straight away so the user know this is working and how it looks like

@tribhuwan-kumar
Copy link
Contributor Author

Screenshot 2025-01-11 at 9 49 24 AM still get this

its because your summary frequency is set to daily and the time you set to send summary is passed way. so, you have to wait till your set time come to get reddit question (this is was already exists in prior pipe, i didn't implemented it)

@tribhuwan-kumar
Copy link
Contributor Author

how can i get the markdown question at the bottom? if there is no logs yet it can just generate a log and markdown or anything that makes generate the question straight away so the user know this is working and how it looks like

for this i've to refactor the whole pipeline!

@tribhuwan-kumar
Copy link
Contributor Author

@louis030195
can it get merge? you can dm me any leftovers. I'll do that in next pr?

@louis030195
Copy link
Collaborator

/approve

my bad could have gave more instructions in the issue

thx for the work!

Copy link

algora-pbc bot commented Jan 14, 2025

@louis030195: The claim has been successfully added to reward-all. You can visit your dashboard to complete the payment.

@louis030195 louis030195 merged commit 4c99d44 into mediar-ai:main Jan 14, 2025
4 of 6 checks passed
@louis030195
Copy link
Collaborator

@louis030195 can it get merge? you can dm me any leftovers. I'll do that in next pr?

still unusable, atm it spams every minute regardless of frequency (which burn lot of AI credits):

image Screenshot 2025-01-16 at 4 07 19 PM

@tribhuwan-kumar
Copy link
Contributor Author

@louis030195 can it get merge? you can dm me any leftovers. I'll do that in next pr?

still unusable, atm it spams every minute regardless of frequency (which burn lot of AI credits):

image Screenshot 2025-01-16 at 4 07 19 PM

fixing it

@tribhuwan-kumar
Copy link
Contributor Author

@louis030195
how are you expecting it to send mails?
if mail is meant to be send on specific time, then what's the point of running cron on every single minute?

@tribhuwan-kumar
Copy link
Contributor Author

i think the email time should be the cron execution time whether its hourly or daily

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[bounty] $100 convert the reddit pipe to nextjs app
2 participants