-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Integrate Upstash Redis for Dynamic PostGroup Storage and Retrieval #12
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
Conversation
WalkthroughReplaces in-file PostGroup sample data with Redis-backed fetch/save in src/postGroup.ts, adds cron/manual title-generation (logTitlesForAllPostGroups), introduces seeding/verify/clear scripts and seed data, and removes a Redis connect console.log in src/redisClient.ts. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor CRON as Cron Scheduler
actor CLI as Manual Runner / seed CLI
participant PG as src/postGroup.ts
participant SD as src/seed.ts
participant SDB as src/seedDatabase.ts
participant RC as src/redisClient.ts
participant RS as Redis (Upstash)
participant TG as Title Generator
participant Log as Console Logger
rect rgb(245,249,252)
CRON->>PG: trigger (every 6h)
CLI->>PG: direct run (require.main)
end
PG->>RC: initRedis()/getRedisClient()
RC->>RS: CONNECT
RS-->>RC: OK
PG->>RS: GET "post-groups"
RS-->>PG: JSON or null
alt data found
PG->>PG: parse -> iterate PostGroups
loop for each PostGroup
PG->>TG: generate title for posts
TG-->>PG: title / error
alt title success
PG->>Log: context-aware log
else
PG->>Log: per-item error log
end
end
PG->>RS: SET updated "post-groups" (if changed)
RS-->>PG: OK
else no data / parse error
PG->>Log: "No PostGroups found in Redis"
end
%% Seed flow
CLI->>SD: run `seed` / `clear` / `verify`
SD->>SDB: dispatch seedDatabase/clearDatabase/verifySeeding
SDB->>RC: initRedis()
SDB->>RS: GET/SET/DEL "post-groups"
RS-->>SDB: OK / data / null
SDB-->>CLI: success / error (exit codes)
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
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/postGroup.ts (2)
49-72
: Persist generated titles back to Redis — current changes are ephemeralYou set group.title in-memory but never persist it; generated titles will be lost. File: src/postGroup.ts (lines 49–72).
export async function logTitlesForAllPostGroups(context: 'CRON' | 'MANUAL' = 'MANUAL') { - const postGroups = await fetchPostGroupsFromRedis(); + let postGroups: PostGroup[]; + try { + postGroups = await fetchPostGroupsFromRedis(); + } catch (e) { + console.error('Failed to fetch PostGroups from Redis:', e); + return; + } if (!postGroups.length) { console.log('No PostGroups found in Redis.'); return; } + const redis = getRedisClient(); + const key = process.env.POST_GROUPS_KEY ?? 'PostGroup'; + let updated = false; for (const group of postGroups) { try { const title = await generateTitleForPostGroup(group); group.title = title; + updated = true; if (context === 'CRON') { console.log(`[CRON] Generated Title for PostGroup (id: ${group.id}) at ${new Date().toISOString()}:`, title); } else { console.log(`Title for PostGroup (id: ${group.id}):`, title); } } catch (e) { if (context === 'CRON') { console.error(`[CRON] Error generating title for PostGroup (id: ${group.id}):`, e); } else { console.error(`Error generating title for PostGroup (id: ${group.id}):`, e); } } } + if (updated) { + // Note: writing the whole array is simple but can clobber concurrent writers. + // Prefer per-group keys or JSON.SET on a path if available. + await redis.set(key, JSON.stringify(postGroups)); + } }If you’re using RedisJSON, switch to per-group keys (e.g.,
pg:{id}
) andJSON.SET pg:{id} $.title "<title>"
to avoid read‑modify‑write races.
74-77
: Guard cron registration, add UTC timezone, and catch top-level errors.src/postGroup.ts schedules a cron at import (lines ~74–77), which registers a job in every process that imports the module. Replace the unconditional schedule with an explicit registration pattern: export a registerCron() (or gate registration via require.main === module or an env var like ENABLE_CRON=true) and call it from your chosen entrypoint. Add { timezone: 'Etc/UTC' } to cron.schedule and wrap the handler in try/catch that logs failures. If deployed serverless, use an external scheduler (Upstash QStash/Cloud Scheduler/GHA) instead of in-process cron.
🧹 Nitpick comments (2)
src/postGroup.ts (2)
31-44
: Harden Redis read: configurable key + schema guard on parsed data.Avoid magic key, validate JSON is an array, and fail gracefully.
Apply this diff:
export async function fetchPostGroupsFromRedis(): Promise<PostGroup[]> { await initRedis(); const redis = getRedisClient(); - // Adjust the key if you use a different one - const data = await redis.get('PostGroup'); - if (!data) return []; - try { - return JSON.parse(data); - } catch (e) { - console.error('Failed to parse PostGroup data from Redis:', e); - return []; - } + const key = process.env.POST_GROUPS_KEY ?? 'PostGroup'; + const raw = await redis.get(key); + if (!raw) return []; + try { + const parsed: unknown = JSON.parse(raw); + if (!Array.isArray(parsed)) { + console.error(`Expected an array at Redis key "${key}", got`, typeof parsed); + return []; + } + return parsed as PostGroup[]; + } catch (e) { + console.error(`Failed to parse PostGroup data from Redis key "${key}":`, e); + return []; + } }Please confirm the actual Redis key name you intend to use in prod/staging via
POST_GROUPS_KEY
.
79-82
: Await manual run to completion and exit cleanly; also ESM‑safe alternative may be needed.The current call isn’t awaited, so the process may exit early (or be kept alive by the cron). Ensure completion and clean exit.
Apply this diff:
// If this file is run directly, generate and log the title for all PostGroups if (require.main === module) { - logTitlesForAllPostGroups('MANUAL'); + logTitlesForAllPostGroups('MANUAL') + .then(() => process.exit(0)) + .catch((e) => { + console.error(e); + process.exit(1); + }); }If the project uses ES modules, replace
require.main === module
with animport.meta.url
check in the entry file.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/postGroup.ts
(2 hunks)src/redisClient.ts
(0 hunks)
💤 Files with no reviewable changes (1)
- src/redisClient.ts
🧰 Additional context used
🧬 Code graph analysis (1)
src/postGroup.ts (1)
src/redisClient.ts (2)
initRedis
(25-83)getRedisClient
(85-91)
🔇 Additional comments (1)
src/postGroup.ts (1)
24-28
: Cap aggregated content to avoid excessive token/cost and timeouts.Limit the combined content size or number of posts before calling
generateTitleForPost
to guard against very large groups.[ suggest_recommended_refactor ]
Apply this diff:export async function generateTitleForPostGroup(postGroup: PostGroup): Promise<string> { - const combinedContent = postGroup.posts.map(post => post.content).join('\n\n'); - return await generateTitleForPost(combinedContent); + // Limit to first N posts and cap characters to keep prompt bounded + const combinedContent = postGroup.posts + .slice(0, 10) + .map(post => post.content) + .join('\n\n') + .slice(0, 4000); + return await generateTitleForPost(combinedContent); }
…d command-line support
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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/postGroup.ts (1)
95-103
: Do not schedule cron on import. Gate behind an env flag and/or move to a dedicated runner.Importing this module anywhere starts the cron, which will happen during seeding/tests and can keep processes alive. Gate it or extract to a separate file.
-// Schedule a cron job to generate and log the title for all PostGroups every 6 hours -cron.schedule('0 */6 * * *', async () => { - await logTitlesForAllPostGroups('CRON'); -}); +// Schedule a cron job only when explicitly enabled (avoid side effects on import) +if (process.env.ENABLE_TITLE_CRON === '1' && require.main === module) { + cron.schedule('0 */6 * * *', async () => { + await logTitlesForAllPostGroups('CRON'); + }); +}Alternatively, move the scheduling into a new src/postGroup.cron.ts that imports logTitlesForAllPostGroups and is invoked by a dedicated npm script.
🧹 Nitpick comments (10)
package.json (1)
14-17
: Seed scripts: good addition; consider a reset alias.Scripts look solid. Optional: add a convenience alias to clear then seed.
"seed:verify": "tsx src/seed.ts verify" + ,"seed:reset": "npm run seed:clear && npm run seed"
src/seedData.ts (1)
3-60
: Static timestamps in seed data.Fine for mock data; if tests depend on recency, consider generating ISO strings at runtime.
src/postGroup.ts (4)
31-36
: Doc comment/key mismatch.Comment says key 'PostGroup' but code uses 'post-groups'. Align to prevent operator confusion.
-// Fetch all PostGroups from Redis (expects a key 'PostGroup' with a JSON array, or adapt as needed) +// Fetch all PostGroups from Redis (expects a key 'post-groups' with a JSON array)
31-44
: Validate Redis payload with zod before use.Blind JSON.parse risks runtime errors/shape drift. You already have zod; use it to parse/validate.
+import { z } from 'zod'; + +const PostSchema = z.object({ + id: z.string(), + content: z.string(), + sentiment: z.enum(["BULLISH","BEARISH","NEUTRAL"]), + source: z.enum(["TWITTER","REDDIT","YOUTUBE","TELEGRAM","FARCASTER"]), + categories: z.array(z.string()), + subcategories: z.array(z.string()), + link: z.string().url().optional(), + createdAt: z.string(), + updatedAt: z.string(), +}); +const PostGroupSchema = z.object({ + id: z.string(), + posts: z.array(PostSchema), + title: z.string().optional(), +}); +const PostGroupsSchema = z.array(PostGroupSchema); + export async function fetchPostGroupsFromRedis(): Promise<PostGroup[]> { await initRedis(); const redis = getRedisClient(); // Adjust the key if you use a different one const data = await redis.get('post-groups'); if (!data) return []; try { - return JSON.parse(data); + return PostGroupsSchema.parse(JSON.parse(data)); } catch (e) { console.error('Failed to parse post-groups data from Redis:', e); return []; } }
56-60
: Type the accumulator.Make intent explicit and keep type safety.
- const postGroupsWithOrderedKeys = []; + const postGroupsWithOrderedKeys: PostGroup[] = [];
48-92
: Optional: reduce OpenAI/API load spikes with bounded concurrency.If groups become large, consider a limiter (e.g., p-limit 3–5) for title generation rather than pure serial or full parallel.
src/seedDatabase.ts (3)
19-23
: Hardcoded Redis key string; centralize.The key 'post-groups' is duplicated across modules. Extract to a shared constant to avoid drift.
Example (new file src/constants.ts):
export const REDIS_KEY_POST_GROUPS = 'post-groups';Then here:
- await redis.set('post-groups', JSON.stringify(seedData)); + await redis.set(REDIS_KEY_POST_GROUPS, JSON.stringify(seedData));
72-80
: Guard JSON.parse with schema or explicit try/catch.A corrupt value will throw. Either reuse the zod schema from src/postGroup.ts or add a dedicated try/catch here.
-const postGroups: PostGroup[] = JSON.parse(data); +let postGroups: PostGroup[]; +try { + postGroups = JSON.parse(data) as PostGroup[]; +} catch (e) { + console.error('❌ Seed verify: invalid JSON in post-groups key:', e); + return; +}
45-55
: Optional: close Redis connections in CLI flows.Not required since seed.ts exits, but graceful disconnect avoids surprises in other callers.
src/seed.ts (1)
1-85
: CLI OK; avoid process.exit if you later add graceful Redis disconnects.Current usage is fine. If you start closing Redis clients, prefer letting the event loop drain instead of forcing exit.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json
(1 hunks)src/postGroup.ts
(3 hunks)src/seed.ts
(1 hunks)src/seedData.ts
(1 hunks)src/seedDatabase.ts
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/seedData.ts (1)
src/postGroup.ts (1)
PostGroup
(18-22)
src/seedDatabase.ts (3)
src/redisClient.ts (2)
initRedis
(25-83)getRedisClient
(85-91)src/seedData.ts (1)
seedData
(3-60)src/postGroup.ts (1)
PostGroup
(18-22)
src/seed.ts (1)
src/seedDatabase.ts (3)
clearDatabase
(45-60)verifySeeding
(65-90)seedDatabase
(9-39)
src/postGroup.ts (1)
src/redisClient.ts (2)
initRedis
(25-83)getRedisClient
(85-91)
@@ -0,0 +1,60 @@ | |||
import { PostGroup } from './postGroup'; |
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.
Importing a type from a module with side effects will start the cron scheduler. Use a type‑only import (or move types).
src/postGroup.ts registers a cron job at import time; importing it here just for typing will trigger it. Switch to a type‑only import immediately.
-import { PostGroup } from './postGroup';
+import type { PostGroup } from './postGroup';
Follow‑up: long‑term, extract Post/PostGroup to a pure types module (e.g., src/types/post.ts) to avoid any runtime side effects from type imports.
📝 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.
import { PostGroup } from './postGroup'; | |
import type { PostGroup } from './postGroup'; |
🤖 Prompt for AI Agents
In src/seedData.ts at line 1, the current import pulls in src/postGroup.ts which
registers a cron job on module load; change the import to a type-only import so
no runtime code runs: replace the runtime import with a type import (e.g.,
"import type { PostGroup } from './postGroup';") or refactor to import PostGroup
from a new pure types module (e.g., src/types/post.ts) if available, ensuring
that only types are imported and no module side effects are executed.
import { initRedis, getRedisClient } from './redisClient'; | ||
import { seedData } from './seedData'; | ||
import { PostGroup } from './postGroup'; | ||
|
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.
Type import triggers side effects; switch to type‑only.
This import loads src/postGroup.ts at runtime, which starts the cron scheduler. Use a type‑only import (and keep cron gated).
-import { PostGroup } from './postGroup';
+import type { PostGroup } from './postGroup';
📝 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.
import { initRedis, getRedisClient } from './redisClient'; | |
import { seedData } from './seedData'; | |
import { PostGroup } from './postGroup'; | |
import { initRedis, getRedisClient } from './redisClient'; | |
import { seedData } from './seedData'; | |
import type { PostGroup } from './postGroup'; | |
🤖 Prompt for AI Agents
In src/seedDatabase.ts around lines 1 to 4, the current import of PostGroup
causes src/postGroup.ts to be loaded at runtime (starting the cron); change the
import to a type-only import: import type { PostGroup } from './postGroup'; and
remove any other top-level runtime access to postGroup module; if you need
runtime behavior from postGroup, load it dynamically behind the cron gating
(e.g., require or dynamic import inside the gated branch) so the module isn't
imported at module-evaluation time.
import { initRedis, getRedisClient } from './redisClient'; | ||
import { seedData } from './seedData'; | ||
import { PostGroup } from './postGroup'; | ||
|
||
/** | ||
* Seeds the Upstash Redis database with mock data for PostGroups. | ||
* This function will clear existing PostGroup data and populate it with the seed data. | ||
*/ | ||
export async function seedDatabase(): Promise<void> { | ||
try { | ||
console.log('🌱 Starting database seeding...'); | ||
|
||
// Initialize Redis connection | ||
await initRedis(); | ||
const redis = getRedisClient(); | ||
|
||
console.log('✅ Connected to Redis'); | ||
|
||
// Store the seed data as post-groups data in Redis | ||
// Using the new key structure as required | ||
await redis.set('post-groups', JSON.stringify(seedData)); | ||
|
||
console.log(`✅ Successfully seeded ${seedData.length} post group(s) to Redis`); | ||
console.log('📊 Seed data summary:'); | ||
|
||
seedData.forEach((group: PostGroup, index: number) => { | ||
console.log(` Group ${index + 1}: ${group.id} (${group.posts.length} posts)`); | ||
group.posts.forEach((post, postIndex) => { | ||
console.log(` Post ${postIndex + 1}: ${post.id} - ${post.sentiment} (${post.source})`); | ||
}); | ||
}); | ||
|
||
console.log('🎉 Database seeding completed successfully!'); | ||
|
||
} catch (error) { | ||
console.error('❌ Error seeding database:', error); | ||
throw error; | ||
} | ||
} | ||
|
||
/** | ||
* Clears all PostGroup data from the database. | ||
* Use with caution - this will delete all existing post groups! | ||
*/ | ||
export async function clearDatabase(): Promise<void> { | ||
try { | ||
console.log('🧹 Clearing PostGroup data from database...'); | ||
|
||
await initRedis(); | ||
const redis = getRedisClient(); | ||
|
||
await redis.del('post-groups'); | ||
|
||
console.log('✅ Database cleared successfully'); | ||
|
||
} catch (error) { | ||
console.error('❌ Error clearing database:', error); | ||
throw error; | ||
} | ||
} | ||
|
||
/** | ||
* Verifies that the seed data was properly stored by retrieving and logging it. | ||
*/ | ||
export async function verifySeeding(): Promise<void> { | ||
try { | ||
console.log('🔍 Verifying seeded data...'); | ||
|
||
await initRedis(); | ||
const redis = getRedisClient(); | ||
|
||
const data = await redis.get('post-groups'); | ||
|
||
if (!data) { | ||
console.log('❌ No data found in database'); | ||
return; | ||
} | ||
|
||
const postGroups: PostGroup[] = JSON.parse(data); | ||
|
||
console.log(`✅ Found ${postGroups.length} post group(s) in database:`); | ||
postGroups.forEach((group, index) => { | ||
console.log(` Group ${index + 1}: ${group.id} (${group.posts.length} posts)`); | ||
}); | ||
|
||
} catch (error) { | ||
console.error('❌ Error verifying seeded data:', error); | ||
throw error; | ||
} | ||
} No newline at end of 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.
🛠️ Refactor suggestion
🧩 Analysis chain
Find and fix any remaining non type‑only imports from './postGroup'.
Run to verify repo-wide.
🏁 Script executed:
#!/bin/bash
# Flag any value imports (non type-only) from './postGroup'
rg -nP --type=ts -C1 "import\s+\{[^}]*\b(PostGroup|Post)\b[^}]*\}\s+from\s+['\"]\./postGroup['\"]" \
| sed 's/^/NON-TYPE IMPORT: /'
# Show existing type-only imports for confirmation
rg -nP --type=ts -C1 "import\s+type\s+\{[^}]*\b(PostGroup|Post)\b[^}]*\}\s+from\s+['\"]\./postGroup['\"]" \
| sed 's/^/TYPE-ONLY IMPORT: /'
Length of output: 329
Convert './postGroup' imports to type-only
- src/seedData.ts:1 — replace
import { PostGroup } from './postGroup';
withimport type { PostGroup } from './postGroup';
- src/seedDatabase.ts:3 — replace
import { PostGroup } from './postGroup';
withimport type { PostGroup } from './postGroup';
🤖 Prompt for AI Agents
In src/seedDatabase.ts (around line 3) and src/seedData.ts (line 1), the
PostGroup import is a runtime import; replace it with a type-only import so
PostGroup is imported only for types (i.e., change the import statement to a
type-only import for PostGroup from './postGroup') to avoid emitting runtime
import code.
PR Description:
This PR removes all hardcoded PostGroup and Post data from the codebase and fully integrates Upstash Redis for persistent storage and retrieval. Key changes include:
This update ensures the codebase is ready for scalable, production-grade data management with no reliance on in-memory or hardcoded data.
Summary by CodeRabbit
New Features
Refactor
Chores