Skip to content

feat: persistent model caching, premium prompt templates, and UI selectors#1

Open
Mr-Dark-debug wants to merge 1 commit into
mainfrom
feature/model-persistence-prompts
Open

feat: persistent model caching, premium prompt templates, and UI selectors#1
Mr-Dark-debug wants to merge 1 commit into
mainfrom
feature/model-persistence-prompts

Conversation

@Mr-Dark-debug

Copy link
Copy Markdown
Owner

Description

This PR introduces several significant features and optimizations to ReddJSON:

  1. Persistent Model Caching:

    • Cache fetched model lists from OpenRouter and Groq in local storage.
    • Prevents redundant, slow API requests on every settings tab opening, improving load time and protecting API rate limits.
  2. Premium LinkedIn Post Templates:

    • Seeds 4 custom-tailored writing styles:
      • LinkedIn Viral Staircase (highly scannable ladder layout, bold hooks)
      • Narrative Storyteller (engaging problem-solving narrative structure)
      • Actionable Playbook (clear how-to masterclass style)
      • Contrarian Debate (thoughtful analysis of misconceptions/unpopular views)
    • Automatically migrates/seeds on install or update without overwriting custom templates.
  3. Context-Specific Template Switchers (UI/UX):

    • Reddit 'Post' Button Right-Click: Right-click the 'Post' button on Reddit to choose a template.
    • Sidebar History Post Generation: Instantly generate posts from saved History JSON using any prompt template.
  4. General Quality-of-Life Improvements:

    • Dark Mode: High-contrast, premium dark mode auto-detected and toggleable.
    • Export/Import: JSON export/import for history and AI posts history.
    • Search Debouncing: Efficient local-memory search filtering.
    • Escape Key: Quick modal closing.
    • Sidebar Toasts: Non-intrusive feedback indicators.

🤖 Generated with Claude Code

…nd UI selectors

- Cache fetched provider models in local storage to prevent redundant API queries
- Add 4 premium LinkedIn post system prompts (Viral Staircase, Storyteller, How-to Playbook, Contrarian)
- Seed prompts into settings via dynamic migration on install/upgrade
- Add right-click context menus on the 'Post' button on Reddit to select templates on the fly
- Add 'Post' generation button to History list items with an inline template selector
- Implement premium dark mode, export/import, search debouncing, and modal escape key handling

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the ReddJSON extension to version 2.1, introducing a dark mode theme, export/import capabilities for history and AI posts, keyboard shortcuts, JSON filtering options (Full Thread vs. Post Only), and storage size optimizations. The review feedback identifies several critical and medium-severity issues, including a bug where settings can be overwritten with null during history import, a logic flaw in the cache eviction policy for processed post IDs, a portability issue with detached DOM nodes during export, a missing safety guard for uninitialized settings providers, and unhandled promise rejections when communicating with disconnected content scripts.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread sidebar.js
Comment on lines +532 to +537
const merged = [...newEntries, ...existing].slice(0, 50);
await chrome.runtime.sendMessage({
action: 'saveSettings',
settings: currentSettings
});
await chrome.storage.local.set({ reddjson_history: merged });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

critical

Critical Bug: Settings Overwritten with null on History Import

When importing history, the code attempts to save settings by calling chrome.runtime.sendMessage with settings: currentSettings.

However, currentSettings is initialized to null and is only populated when the user visits the Settings tab. If a user imports history immediately after opening the sidebar (without visiting the Settings tab first), currentSettings will be null. This will send null to the background script, which completely overwrites and wipes out all of the user's configured API keys, default models, and custom prompts in chrome.storage.local.

Since saving settings is completely unnecessary during a history import, this call should be removed entirely.

                const merged = [...newEntries, ...existing].slice(0, 50);
                await chrome.storage.local.set({ reddjson_history: merged });

Comment thread sidebar.js
Comment on lines +494 to +502
function exportAsJSON(data, filename) {
const blob = new Blob([JSON.stringify(data, null, 2)], { type: 'application/json' });
const url = URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = `${filename}-${new Date().toISOString().slice(0, 10)}.json`;
a.click();
URL.revokeObjectURL(url);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Portability Issue: Detached DOM Node Click

Calling .click() on a detached <a> element (one that is not appended to the document DOM) fails to trigger a download in several browsers (such as Firefox) and under strict security policies in Chrome.

To ensure cross-browser compatibility and robust download behavior, temporarily append the element to document.body before clicking it, and then remove it immediately afterward.

Suggested change
function exportAsJSON(data, filename) {
const blob = new Blob([JSON.stringify(data, null, 2)], { type: 'application/json' });
const url = URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = `${filename}-${new Date().toISOString().slice(0, 10)}.json`;
a.click();
URL.revokeObjectURL(url);
}
function exportAsJSON(data, filename) {
const blob = new Blob([JSON.stringify(data, null, 2)], { type: 'application/json' });
const url = URL.createObjectURL(blob);
const a = document.createElement('a');
a.href = url;
a.download = `${filename}-${new Date().toISOString().slice(0, 10)}.json`;
document.body.appendChild(a);
a.click();
document.body.removeChild(a);
URL.revokeObjectURL(url);
}

Comment thread sidebar.js
Comment on lines 698 to 699
if (!currentSettings.providers[pid]) currentSettings.providers[pid] = {};
currentSettings.providers[pid].selectedModel = modelId;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Missing Safety Guard for currentSettings.providers

If currentSettings.providers is undefined (e.g., if settings are uninitialized or cleared), accessing currentSettings.providers[pid] will throw a TypeError and crash the click handler.

Add a defensive check to ensure currentSettings.providers is initialized as an object before accessing its properties, similar to how it is handled in other parts of the file.

Suggested change
if (!currentSettings.providers[pid]) currentSettings.providers[pid] = {};
currentSettings.providers[pid].selectedModel = modelId;
if (!currentSettings.providers) currentSettings.providers = {};
if (!currentSettings.providers[pid]) currentSettings.providers[pid] = {};
currentSettings.providers[pid].selectedModel = modelId;

Comment thread content.js
Comment on lines +97 to +101
if (processedPostIds.size >= CONFIG.maxProcessedIds) {
const first = processedPostIds.values().next().value;
processedPostIds.delete(first);
}
processedPostIds.add(id);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Logic Bug in Cache Eviction Policy

In the current implementation of trackPostId, if the added id is already present in the processedPostIds Set and the Set size is at the limit (CONFIG.maxProcessedIds), the code will evict the first element from the Set and then add the id. Since the id is already in the Set, adding it is a no-op, which incorrectly shrinks the Set size by 1.

To fix this and implement a proper FIFO/LRU update, check if the Set already has the id first. If it does, delete it so that re-adding it moves it to the end (refreshing its freshness) without evicting any other elements.

  if (processedPostIds.has(id)) {
    processedPostIds.delete(id);
  } else if (processedPostIds.size >= CONFIG.maxProcessedIds) {
    const first = processedPostIds.values().next().value;
    processedPostIds.delete(first);
  }
  processedPostIds.add(id);

Comment thread background.js
Comment on lines +741 to +744
if (!result.success) {
chrome.tabs.sendMessage(tab.id, { action: 'showToast', message: result.error, type: 'error' });
return;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Unhandled Promise Rejection in chrome.tabs.sendMessage

If the content script is not active, has been unloaded, or is disconnected (e.g., on special Reddit pages or immediately after an extension update), calling chrome.tabs.sendMessage will throw an error: "Could not establish connection. Receiving end does not exist."

Since this is an asynchronous call, leaving it unhandled will trigger an unhandled promise rejection. Add a .catch() block to handle this gracefully.

Suggested change
if (!result.success) {
chrome.tabs.sendMessage(tab.id, { action: 'showToast', message: result.error, type: 'error' });
return;
}
if (!result.success) {
chrome.tabs.sendMessage(tab.id, { action: 'showToast', message: result.error, type: 'error' }).catch(() => {});
return;
}

Comment thread background.js
Comment on lines +746 to +747
const pretty = JSON.stringify(result.data, null, 2);
chrome.tabs.sendMessage(tab.id, { action: 'copyToClipboard', text: pretty });

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Unhandled Promise Rejection in chrome.tabs.sendMessage

Similar to other message sending calls, if the content script is disconnected or not loaded, chrome.tabs.sendMessage will throw an error. Add a .catch() block to prevent unhandled promise rejections.

Suggested change
const pretty = JSON.stringify(result.data, null, 2);
chrome.tabs.sendMessage(tab.id, { action: 'copyToClipboard', text: pretty });
const pretty = JSON.stringify(result.data, null, 2);
chrome.tabs.sendMessage(tab.id, { action: 'copyToClipboard', text: pretty }).catch(() => {});

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant