feat: Add Sentiment-Based Summaries to PostGroup and Store in Redis #13
Conversation
…gging functionality
WalkthroughAdds sentiment-summary generation: new SentimentSummaries type and generator using validated OpenAI output; PostGroup gains optional bullish/bearish/neutral fields and a wrapper to generate and persist these summaries alongside titles; callOpenAIWithValidation gains an optional maxTokens parameter. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Runner as Cron/CLI
participant PG as PostGroup Manager
participant Gen as generateSentimentSummariesForGroup
participant LLM as OpenAI
participant DB as Redis
Runner->>PG: logTitlesForAllPostGroups()
PG->>DB: fetch PostGroups
loop per PostGroup
PG->>PG: generate titles (existing)
PG->>Gen: generateSentimentSummariesForPostGroup(group)
Gen->>LLM: send system+user prompts (max_tokens optional)
LLM-->>Gen: JSON {bullishSummary?, bearishSummary?, neutralSummary?}
Gen->>Gen: validate via SentimentSummariesSchema
Gen-->>PG: SentimentSummaries (at least one present)
alt any summary changed
PG->>DB: save PostGroup (titles + summaries)
PG->>Runner: log saved titles & summaries
else
PG->>Runner: log no changes
end
end
PG-->>Runner: done
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (7)
src/generateTitle.ts (3)
24-30: Harden validation: trim and reject unknown keys.Prevents whitespace-only strings and silently-accepted extra keys.
-const SentimentSummariesSchema = z.object({ - bullishSummary: z.string().min(10, "Bullish summary must be at least 10 characters").max(500, "Bullish summary must not exceed 500 characters"), - bearishSummary: z.string().min(10, "Bearish summary must be at least 10 characters").max(500, "Bearish summary must not exceed 500 characters"), - neutralSummary: z.string().min(10, "Neutral summary must be at least 10 characters").max(500, "Neutral summary must not exceed 500 characters") -}); +const SentimentSummariesSchema = z.object({ + bullishSummary: z.string().trim().min(10, "Bullish summary must be at least 10 characters").max(500, "Bullish summary must not exceed 500 characters"), + bearishSummary: z.string().trim().min(10, "Bearish summary must be at least 10 characters").max(500, "Bearish summary must not exceed 500 characters"), + neutralSummary: z.string().trim().min(10, "Neutral summary must be at least 10 characters").max(500, "Neutral summary must not exceed 500 characters") +}).strict();
69-69: Avoidany[]and send only minimal, sanitized fields to the model.Cuts token usage/cost and reduces data exposure surface.
-export async function generateSentimentSummariesForGroup(posts: any[], clientOverride?: OpenAI): Promise<SentimentSummaries> { +export async function generateSentimentSummariesForGroup( + posts: Array<{ content: string; sentiment: "BULLISH" | "BEARISH" | "NEUTRAL" }>, + clientOverride?: OpenAI +): Promise<SentimentSummaries> { @@ - const postsJson = JSON.stringify(posts); + const minimal = posts.map(p => ({ content: p.content, sentiment: p.sentiment })); + const postsJson = JSON.stringify(minimal);Also applies to: 92-99
103-110: Redundant re-check and re-wrap; just return the parsed object.Zod guarantees presence/shape already.
- if (!validated?.bullishSummary || !validated?.bearishSummary || !validated?.neutralSummary) { - throw new Error(`Sentiment summaries generation failed for posts: ${postsJson}`); - } - return { - bullishSummary: validated.bullishSummary, - bearishSummary: validated.bearishSummary, - neutralSummary: validated.neutralSummary - }; + return validated;src/postGroup.ts (4)
33-36: Pass a minimal shape to the generator to reduce tokens/cost.Avoid shipping full Post objects to the LLM.
-export async function generateSentimentSummariesForPostGroup(postGroup: PostGroup): Promise<SentimentSummaries> { - return await generateSentimentSummariesForGroup(postGroup.posts); -} +export async function generateSentimentSummariesForPostGroup(postGroup: PostGroup): Promise<SentimentSummaries> { + const minimal = postGroup.posts.map(p => ({ content: p.content, sentiment: p.sentiment })); + return await generateSentimentSummariesForGroup(minimal); +}
78-87: Good change detection; consider gating to save tokens.Optional: skip regeneration when existing summaries are present and posts unchanged (hash of contents), or behind a feature flag.
110-119: Whole-key overwrite; consider TTL and contention safeguards.Setting the entire array risks lost updates if multiple writers. Consider:
- A Redis hash per group (HSET) or JSON.* updates; or
- Optimistic locking (WATCH/MULTI) or versioning; and
- Optional TTL if these groups are ephemeral.
Example (TTL only):
- await redis.set('post-groups', JSON.stringify(postGroupsWithOrderedKeys)); + await redis.set('post-groups', JSON.stringify(postGroupsWithOrderedKeys), { EX: 60 * 60 * 24 });
124-126: Comment drift.This cron now also generates summaries. Update the comment.
-// Schedule a cron job to generate and log the title for all PostGroups every 6 hours +// Schedule a cron job to generate titles and sentiment summaries for all PostGroups every 6 hours
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/generateTitle.ts(2 hunks)src/postGroup.ts(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/postGroup.ts (2)
src/generateTitle.ts (2)
SentimentSummaries(18-22)generateSentimentSummariesForGroup(69-115)src/redisClient.ts (2)
initRedis(25-83)getRedisClient(85-91)
src/generateTitle.ts (1)
src/openaiValidationUtil.ts (1)
callOpenAIWithValidation(10-71)
🔇 Additional comments (6)
src/generateTitle.ts (1)
17-23: Type looks good.Clear, explicit fields; matches downstream usage.
src/postGroup.ts (5)
3-3: Import changes look good.
22-24: PostGroup type extension LGTM.Optional fields are appropriate and backwards-compatible.
67-70: State wiring LGTM.Keeps prior values unless updates land.
96-99: Error logs avoid printing summaries—good.Consistent with objective to not print summaries.
56-119: Verify logs don't print sentiment summary contents
- No console logs directly referencing bullishSummary / bearishSummary / neutralSummary were found.
- Check src/seedDatabase.ts (≈lines 22–26): prints "📊 Seed data summary:" and iterates seedData — ensure the loop does not console.log full group objects or summary fields (remove/redact if it does).
- src/postGroup.ts (lines 114–120): logs only a status message ("...sentiment summaries saved to Redis.") — acceptable if it doesn't include the summaries' content.
…ing and maxTokens parameter
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
src/openaiValidationUtil.ts (3)
37-42: Reduce invalid JSON and retries: add temperature=0 and scale max_tokens on retries.This makes outputs more deterministic and prevents repeated truncation when the schema has multi‑field summaries.
Apply this diff:
- const chatParams = { - model: "gpt-4o-mini", - messages, - response_format: { type: "json_object" } as const, - max_tokens: params.maxTokens ?? 200 - }; + const baseMaxTokens = params.maxTokens ?? 200; + const maxTokensForAttempt = Math.min( + Math.floor(baseMaxTokens * Math.pow(2, attempt - 1)), + 2048 + ); + const chatParams = { + model: "gpt-4o-mini", + messages, + response_format: { type: "json_object" } as const, + temperature: 0, + max_tokens: maxTokensForAttempt, + };
6-9: Docstring contradicts behavior (throws vs returns null).Function throws after exhausting retries; it never returns null. Fix the comment.
Apply this diff:
- * Returns the validated result or null if all attempts fail. + * Returns the validated result or throws after all attempts fail.
15-17: maxTokens param addition is solid; guard against truncation in long JSON responses.Given the new multi‑summary use cases, 200 tokens may be too tight and can yield truncated JSON. Recommend scaling tokens per retry and setting a deterministic temperature in the request (see diff on Lines 37–42).
Run to see current callers and whether they pass an explicit
maxTokens:#!/bin/bash rg -n --type=ts -C2 '\bcallOpenAIWithValidation\s*\('
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/generateTitle.ts(2 hunks)src/openaiValidationUtil.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/generateTitle.ts
…eration logic for PostGroups
There was a problem hiding this comment.
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/generateTitle.ts (1)
63-64: Avoid logging raw post content (privacy/compliance).Logging the full post can leak sensitive data; prefer metadata only.
Apply:
- console.error("Error generating title for post:", post, e); + console.error("Error generating title for post.", { + charCount: typeof post === 'string' ? post.length : undefined, + error: e instanceof Error ? e.message : String(e) + });
♻️ Duplicate comments (2)
src/generateTitle.ts (2)
91-99: Token limit override implemented.Plumbing
maxTokens: 800addresses prior output truncation risk.
111-114: Good: error logging avoids raw payload.Logging count + message only addresses the earlier privacy concern.
🧹 Nitpick comments (6)
src/generateTitle.ts (3)
68-71: Tighten input typing to what the function actually needs.Use a minimal structural type to avoid
any[]and reduce coupling.-export async function generateSentimentSummariesForGroup(posts: any[], clientOverride?: OpenAI): Promise<SentimentSummaries> { +type MinimalPost = { content: string; sentiment: "BULLISH" | "BEARISH" | "NEUTRAL" }; +export async function generateSentimentSummariesForGroup(posts: MinimalPost[], clientOverride?: OpenAI): Promise<SentimentSummaries> {
72-88: Prompt hardening: forbid PII/handles in summaries.Add an explicit instruction to avoid leaking identifiers.
Instructions: @@ -5. Make summaries informative and actionable, but do not make up content for sentiments not present in the posts. +5. Make summaries informative and actionable, but do not make up content for sentiments not present in the posts. +6. Do not include usernames/handles, links, or any PII; summarize themes only. -6. Return only valid JSON in this format, including only the summaries for sentiments that exist in the posts: +7. Return only valid JSON in this format, including only the summaries for sentiments that exist in the posts:
101-104: Graceful no-op when no summaries are present.Empty input or sentiment absence shouldn’t be an error; return
{}to avoid failing the whole group.- if (!validated?.bullishSummary && !validated?.bearishSummary && !validated?.neutralSummary) { - throw new Error(`Sentiment summaries generation failed for posts: no summaries returned.`); - } + if (!validated?.bullishSummary && !validated?.bearishSummary && !validated?.neutralSummary) { + return {}; + }src/postGroup.ts (3)
93-101: Minor: redundant initRedis before saving.Client is already initialized earlier; reuse the existing connection.
- await initRedis(); - const redis = getRedisClient(); + const redis = getRedisClient();
56-101: Consider bounded concurrency for OpenAI calls.Sequential processing will be slow with many groups; use a small concurrency limiter (e.g., 3–5) to balance throughput and rate limits.
I can provide a
p-limitbased refactor if desired.
109-112: ESM/CJS main-module check may break under ESM.
require.mainis undefined in pure ESM. Verify module type and switch to an ESM-safe check.Option (ESM):
// add at top: import { pathToFileURL } from 'url'; if (import.meta.url === pathToFileURL(process.argv[1]!).href) { logTitlesForAllPostGroups('MANUAL'); }Option (CJS): keep current code. Which module system is this built with?
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/generateTitle.ts(2 hunks)src/postGroup.ts(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/postGroup.ts (2)
src/generateTitle.ts (2)
SentimentSummaries(18-22)generateSentimentSummariesForGroup(69-117)src/redisClient.ts (2)
initRedis(25-83)getRedisClient(85-91)
src/generateTitle.ts (1)
src/openaiValidationUtil.ts (1)
callOpenAIWithValidation(10-72)
🔇 Additional comments (6)
src/generateTitle.ts (3)
17-23: Type surface for sentiment summaries looks good.Optional fields align with intended “only-when-present” behavior.
24-30: Schema matches the type and prompt constraints.Min/max bounds are reasonable for concise summaries.
35-48: Title system prompt tweak LGTM.Finance/tech context and strict JSON instruction are appropriate.
src/postGroup.ts (3)
3-3: Imports updated correctly for summaries.Surface looks consistent with generateTitle exports.
22-24: PostGroup extended with optional summaries — OK.Matches storage/logging behavior (no summaries printed).
34-36: Wrapper for group summaries is fine.Delegation keeps API tidy.
| const postsJson = JSON.stringify(posts); | ||
|
|
There was a problem hiding this comment.
Send only the minimum data needed to the LLM.
Reduce payload to content + sentiment to lower cost/PII exposure.
- const postsJson = JSON.stringify(posts);
+ const minimal = Array.isArray(posts)
+ ? posts.map(p => ({ content: p?.content, sentiment: p?.sentiment }))
+ : [];
+ const postsJson = JSON.stringify(minimal);📝 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 postsJson = JSON.stringify(posts); | |
| const minimal = Array.isArray(posts) | |
| ? posts.map(p => ({ content: p?.content, sentiment: p?.sentiment })) | |
| : []; | |
| const postsJson = JSON.stringify(minimal); |
🤖 Prompt for AI Agents
In src/generateTitle.ts around lines 89-90, the code currently serializes the
entire posts objects (const postsJson = JSON.stringify(posts)); reduce payload
by mapping posts to only the fields required by the LLM (content and sentiment)
and then JSON.stringify that reduced array before sending; update the variable
to reflect the minimal payload and ensure any downstream usage expects the
reduced shape.
| postGroupsWithOrderedKeys.push({ | ||
| id: group.id, | ||
| title, | ||
| bullishSummary: summaries.bullishSummary, | ||
| bearishSummary: summaries.bearishSummary, | ||
| neutralSummary: summaries.neutralSummary, | ||
| posts: group.posts | ||
| }); |
There was a problem hiding this comment.
Preserve all existing group fields when saving.
Current push constructs a new object and may drop unmodeled fields. Merge into the original to avoid data loss.
- postGroupsWithOrderedKeys.push({
- id: group.id,
- title,
- bullishSummary: summaries.bullishSummary,
- bearishSummary: summaries.bearishSummary,
- neutralSummary: summaries.neutralSummary,
- posts: group.posts
- });
+ const updatedGroup: PostGroup = {
+ ...group,
+ title,
+ ...summaries
+ };
+ postGroupsWithOrderedKeys.push(updatedGroup);📝 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.
| postGroupsWithOrderedKeys.push({ | |
| id: group.id, | |
| title, | |
| bullishSummary: summaries.bullishSummary, | |
| bearishSummary: summaries.bearishSummary, | |
| neutralSummary: summaries.neutralSummary, | |
| posts: group.posts | |
| }); | |
| const updatedGroup: PostGroup = { | |
| ...group, | |
| title, | |
| ...summaries | |
| }; | |
| postGroupsWithOrderedKeys.push(updatedGroup); |
🤖 Prompt for AI Agents
In src/postGroup.ts around lines 77 to 84, the code builds a new object literal
which can drop unmodeled fields on the original group; instead merge the
original group into the pushed object so all existing properties are preserved
and only the intended fields are overridden. Replace the explicit object
construction with a merged form (e.g. spread or Object.assign) that starts from
group and then sets title, bullishSummary, bearishSummary, neutralSummary and
posts so no original fields are lost.
| } catch (e) { | ||
| if (context === 'CRON') { | ||
| console.error(`[CRON] Error generating title for PostGroup (id: ${group.id}):`, e); | ||
| console.error(`[CRON] Error generating title/summaries for PostGroup (id: ${group.id}):`, e); | ||
| } else { | ||
| console.error(`Error generating title for PostGroup (id: ${group.id}):`, e); | ||
| console.error(`Error generating title/summaries for PostGroup (id: ${group.id}):`, e); | ||
| } | ||
| } |
There was a problem hiding this comment.
Do not drop groups on generation failure.
On error, the group is omitted from the saved array, effectively deleting it from Redis.
} catch (e) {
if (context === 'CRON') {
console.error(`[CRON] Error generating title/summaries for PostGroup (id: ${group.id}):`, e);
} else {
console.error(`Error generating title/summaries for PostGroup (id: ${group.id}):`, e);
}
+ // Preserve existing group to avoid data loss
+ postGroupsWithOrderedKeys.push(group);
}📝 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.
| } catch (e) { | |
| if (context === 'CRON') { | |
| console.error(`[CRON] Error generating title for PostGroup (id: ${group.id}):`, e); | |
| console.error(`[CRON] Error generating title/summaries for PostGroup (id: ${group.id}):`, e); | |
| } else { | |
| console.error(`Error generating title for PostGroup (id: ${group.id}):`, e); | |
| console.error(`Error generating title/summaries for PostGroup (id: ${group.id}):`, e); | |
| } | |
| } | |
| } catch (e) { | |
| if (context === 'CRON') { | |
| console.error(`[CRON] Error generating title/summaries for PostGroup (id: ${group.id}):`, e); | |
| } else { | |
| console.error(`Error generating title/summaries for PostGroup (id: ${group.id}):`, e); | |
| } | |
| // Preserve existing group to avoid data loss | |
| postGroupsWithOrderedKeys.push(group); | |
| } |
🤖 Prompt for AI Agents
In src/postGroup.ts around lines 85 to 91, the current catch swallows generation
errors and lets the group be omitted from the saved array (effectively deleting
it from Redis); ensure the group is always preserved by adding logic to append
the original group to the saved list even when generation fails — either move
the savedGroups.push(group) into a finally block that runs regardless of
success/failure, or explicitly push the group inside the catch before logging
the error so the group is retained and saved back to Redis.
Description:
This PR introduces sentiment-based summaries (bullish, bearish, neutral) for each PostGroup. The following changes are included:
bullishSummary,bearishSummary, andneutralSummaryfields.Summary by CodeRabbit
New Features
Chores