fix: increase maxTokens and prevent markdown wrapping in timeline lor… #76
Conversation
…e JSON generation
Timeline lore summary generation was failing due to two issues:
1. maxTokens limit of 480 was too small, causing JSON responses to be truncated
2. Model was returning markdown-wrapped JSON (```json...```), adding overhead
Changes:
- Increased maxTokens from 480 to 1000 to allow complete JSON responses
- Updated prompt to explicitly request raw JSON without markdown formatting
- Added clear instruction to start with { and end with }
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
WalkthroughEnhanced JSON output formatting in prompts with explicit raw JSON boundaries. Increased token budget for timeline summary generation from 480 to 1000 tokens. Added JSON extraction and normalization parsing flow to post-process model outputs. Changes
Sequence Diagram(s)sequenceDiagram
participant Model as Language Model
participant Parse as JSON Parser
participant Normalize as Normalizer
Note over Model: Model output<br/>(raw JSON format<br/>starts with { ends with })
Model->>Parse: Raw JSON string
activate Parse
Parse->>Parse: _extractJsonObject()
Parse-->>Normalize: Extracted object
deactivate Parse
activate Normalize
Normalize->>Normalize: _normalizeTimelineLoreDigest()
Normalize->>Normalize: Adjust tags, insights<br/>based on ranked context
Normalize-->>Normalize: Return normalized digest
deactivate Normalize
Note over Normalize: Final processed output<br/>(normalized fields)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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.
Pull Request Overview
This PR refines the prompt instructions for JSON response generation in the Nostr plugin's summary creation service. The changes aim to prevent the language model from wrapping JSON responses in markdown code fences.
- Updated prompt instructions to explicitly prohibit markdown formatting
- Increased token limit from 480 to 1000 to accommodate responses
- Added clearer emphasis on raw JSON output format
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type, | ||
| prompt, | ||
| { maxTokens: 480, temperature: 0.45 }, | ||
| { maxTokens: 1000, temperature: 0.45 }, |
There was a problem hiding this comment.
The maxTokens increase from 480 to 1000 represents a 108% increase. This change could impact API costs and response time significantly. Consider whether the full 1000 tokens are necessary, or if a more conservative value (e.g., 600-750) would suffice for JSON responses that typically don't require such large limits.
| { maxTokens: 1000, temperature: 0.45 }, | |
| { maxTokens: 750, temperature: 0.45 }, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
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 (1)
plugin-nostr/lib/service.js (1)
6859-6865: Undefined timelineLoreMaxPostsInPrompt can break prompt sizingtimelineLoreMaxPostsInPrompt is used to cap posts in the prompt but is never initialized in the constructor. Math.min(undefined, n) → NaN; slice(-NaN) coerces to 0, likely pulling the entire batch and ballooning prompts.
Fix by setting a sane default in the constructor (and optionally guard where used):
@@ this.timelineLoreBuffer = []; this.timelineLoreMaxBuffer = 120; - this.timelineLoreBatchSize = 50; + this.timelineLoreBatchSize = 50; + // Max posts we include in the lore prompt (configurable) + this.timelineLoreMaxPostsInPrompt = Math.max( + 8, + Math.min( + 60, + Number(this.runtime?.getSetting?.('CTX_TIMELINE_LORE_MAX_POSTS_IN_PROMPT') ?? '30') + ) + );And add a defensive guard at use site:
- const maxPostsInPrompt = Math.min(this.timelineLoreMaxPostsInPrompt, batch.length); + const cap = Number.isFinite(this.timelineLoreMaxPostsInPrompt) ? this.timelineLoreMaxPostsInPrompt : 30; + const maxPostsInPrompt = Math.min(Math.max(8, cap), Math.max(1, batch.length));
🧹 Nitpick comments (2)
plugin-nostr/lib/service.js (2)
6996-7045: Brace-scan JSON extraction can mis-detect inside strings (optional hardening)_current approach counts all “{”/“}” without considering quoted strings/escapes; rare but can misparse. Consider a lightweight state machine that ignores braces inside strings.
6949-6956: Add character budget guard to prevent excessive prompt sizes beyond post count limits
timelineLoreMaxPostsInPromptalready restricts post quantity, but large posts can still balloon the prompt beyond provider limits. The suggested character budget (6000 chars for posts section) is a practical safeguard. While a warning exists, enforcing the budget upfront is cleaner:@@ - const postLines = recentBatch.map((item, idx) => { + let remaining = 6000; // rough prompt budget for posts section + const postLines = []; + for (let idx = 0; idx < recentBatch.length; idx++) { + const item = recentBatch[idx]; const shortAuthor = item.pubkey ? `${item.pubkey.slice(0, 8)}…` : 'unknown'; const cleanContent = this._stripHtmlForLore(item.content || ''); const rationale = this._coerceLoreString(item.rationale || 'signal'); const signalLine = this._coerceLoreStringArray(item.metadata?.signals || [], 4).join('; ') || 'no explicit signals'; - - return [ + const block = [ `[#${idx + 1}] Author: ${shortAuthor} • Score: ${typeof item.score === 'number' ? item.score.toFixed(2) : 'n/a'} • Importance: ${item.importance}`, `CONTENT: ${cleanContent}`, `RATIONALE: ${rationale}`, `SIGNALS: ${signalLine}`, - ].join('\n'); - }).join('\n\n'); + ].join('\n'); + if (remaining - block.length < 0) break; + remaining -= block.length + 2; + postLines.push(block); + } + const postLinesStr = postLines.join('\n\n'); @@ -POSTS TO ANALYZE (${recentBatch.length} posts): -${postLines} +POSTS TO ANALYZE (${postLines.length} posts): +${postLinesStr}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugin-nostr/lib/service.js(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
plugin-nostr/lib/service.js (2)
plugin-nostr/lib/generation.js (1)
raw(7-7)plugin-nostr/debug-text-generation.js (1)
raw(118-118)
🔇 Additional comments (1)
plugin-nostr/lib/service.js (1)
6929-6947: Prompt hardening for RAW JSON is solidClear “RAW JSON ONLY” and explicit “start with { / end with }” constraints reduce fence/overhead issues and improve parse reliability. LGTM.
…e JSON generation (#76) Timeline lore summary generation was failing due to two issues: 1. maxTokens limit of 480 was too small, causing JSON responses to be truncated 2. Model was returning markdown-wrapped JSON (```json...```), adding overhead Changes: - Increased maxTokens from 480 to 1000 to allow complete JSON responses - Updated prompt to explicitly request raw JSON without markdown formatting - Added clear instruction to start with { and end with } 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
…e JSON generation
Timeline lore summary generation was failing due to two issues:
json...), adding overheadChanges:
🤖 Generated with Claude Code
Summary by CodeRabbit