Skip to content

added bug #5

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.

Sign up for GitHub

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

Open
dachakra-coderabbit wants to merge 3 commits into
base: main
Choose a base branch
from
Open

added bug #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e19617
added bug
dachakra-coderabbit Feb 10, 2026
d7334eb
Fix bug
dachakra-coderabbit Feb 10, 2026
8fd8bf1
Fix all the bugs
dachakra-coderabbit Feb 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
316 changes: 313 additions & 3 deletions extension/filter-utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,9 @@ export function extractTitles(data) {
url: issue.url,
prNumber: pr.number,
prTitle: pr.title,
priority: issue.priority
priority: issue.priority,
accepted: issue.accepted || false,
acceptanceMethod: issue.acceptanceMethod || null
});
});
});
Expand Down Expand Up @@ -215,6 +217,185 @@ export function applyPriorityFilter(currentData, selectedPriorities, displayTitl
displayTitlesCallback('commentTitles', filteredTitles, selectedPriorities);
}

/**
* Initializes acceptance filter UI and event handlers
* @param {Object} data - The PR analysis data
* @param {string} currentStatus - Currently selected acceptance status ('all', 'accepted', or 'not-accepted')
* @param {Function} onFilterChange - Callback when filter changes, receives new status as parameter
*/
export function initializeAcceptanceFilter(data, currentStatus, onFilterChange) {
// Count accepted and not accepted issues
let acceptedCount = 0;
let notAcceptedCount = 0;

data.pullRequests.forEach(pr => {
pr.actionableIssues.forEach(issue => {
if (issue.accepted) {
acceptedCount++;
} else {
notAcceptedCount++;
}
});
});

const totalCount = acceptedCount + notAcceptedCount;

// Show the filter section
const filterSection = document.getElementById('acceptanceFilterSection');
if (filterSection) {
filterSection.style.display = 'block';

// Update counts
const allCountEl = document.getElementById('acceptanceCountAll');
const acceptedCountEl = document.getElementById('acceptanceCountAccepted');
const notAcceptedCountEl = document.getElementById('acceptanceCountNotAccepted');

if (allCountEl) allCountEl.textContent = totalCount;
if (acceptedCountEl) acceptedCountEl.textContent = acceptedCount;
if (notAcceptedCountEl) notAcceptedCountEl.textContent = notAcceptedCount;

// Add click handlers to filter buttons
const controlsContainer = document.getElementById('acceptanceFilterControls');
if (controlsContainer) {
controlsContainer.querySelectorAll('.acceptance-filter-btn').forEach(button => {
button.addEventListener('click', () => {
const newStatus = button.getAttribute('data-acceptance');

// Update button states
controlsContainer.querySelectorAll('.acceptance-filter-btn').forEach(btn => {
btn.classList.toggle('active', btn.getAttribute('data-acceptance') === newStatus);
});

// Call the filter change callback with new status
onFilterChange(newStatus);
});
});
}
}
}

/**
* Updates filter counts based on current filter state
* @param {Object} currentData - The current PR data
* @param {Set} selectedPriorities - Set of selected priorities
* @param {string} selectedAcceptanceStatus - Selected acceptance status
*/
export function updateFilterCounts(currentData, selectedPriorities, selectedAcceptanceStatus) {
if (!currentData) return;

// Count items for each priority (filtered by acceptance)
const priorityCounts = {};
let totalWithAcceptance = 0;

currentData.pullRequests.forEach(pr => {
pr.actionableIssues.forEach(issue => {
// Check if this issue matches the acceptance filter
let matchesAcceptance = true;
if (selectedAcceptanceStatus === 'accepted') {
matchesAcceptance = issue.accepted;
} else if (selectedAcceptanceStatus === 'not-accepted') {
matchesAcceptance = !issue.accepted;
}

if (matchesAcceptance) {
const priority = issue.priority || 'Unknown';
priorityCounts[priority] = (priorityCounts[priority] || 0) + 1;
totalWithAcceptance++;
}
});
});

// Update priority filter counts
const priorityAllCount = document.getElementById('priorityCountAll');
if (priorityAllCount) {
priorityAllCount.textContent = totalWithAcceptance;
}

Object.entries(priorityCounts).forEach(([priority, count]) => {
const button = document.querySelector(`[data-priority="${priority}"] .priority-count`);
if (button) {
button.textContent = count;
}
});

// Count items for acceptance status (filtered by priority)
let acceptedCount = 0;
let notAcceptedCount = 0;
let totalWithPriority = 0;

currentData.pullRequests.forEach(pr => {
pr.actionableIssues.forEach(issue => {
// Check if this issue matches the priority filter
const matchesPriority = selectedPriorities.has('all') || selectedPriorities.has(issue.priority);

if (matchesPriority) {
totalWithPriority++;
if (issue.accepted) {
acceptedCount++;
} else {
notAcceptedCount++;
}
}
});
});

// Update acceptance filter counts
const acceptanceAllCount = document.getElementById('acceptanceCountAll');
const acceptanceAcceptedCount = document.getElementById('acceptanceCountAccepted');
const acceptanceNotAcceptedCount = document.getElementById('acceptanceCountNotAccepted');

if (acceptanceAllCount) acceptanceAllCount.textContent = totalWithPriority;
if (acceptanceAcceptedCount) acceptanceAcceptedCount.textContent = acceptedCount;
if (acceptanceNotAcceptedCount) acceptanceNotAcceptedCount.textContent = notAcceptedCount;
}

/**
* Applies both priority and acceptance filters and displays filtered titles
* @param {Object} currentData - The current PR data
* @param {Set} selectedPriorities - Set of selected priorities
* @param {string} selectedAcceptanceStatus - Selected acceptance status ('all', 'accepted', or 'not-accepted')
* @param {Function} displayTitlesCallback - Function to display titles
*/
export function applyCombinedFilters(currentData, selectedPriorities, selectedAcceptanceStatus, displayTitlesCallback) {
if (!currentData) return;

// Update filter counts based on current state
updateFilterCounts(currentData, selectedPriorities, selectedAcceptanceStatus);

// Extract and group titles
const titles = extractTitles(currentData);

// Apply both filters together with AND logic
let filteredTitles = titles.map(group => {
// Filter occurrences by both priority AND acceptance status
const filteredOccurrences = group.allOccurrences.filter(occurrence => {
// Check priority filter
const matchesPriority = selectedPriorities.has('all') || selectedPriorities.has(occurrence.priority);

// Check acceptance filter
let matchesAcceptance = true;
if (selectedAcceptanceStatus === 'accepted') {
matchesAcceptance = occurrence.accepted;
} else if (selectedAcceptanceStatus === 'not-accepted') {
matchesAcceptance = !occurrence.accepted;
}

// Both filters must match (AND logic)
return matchesPriority && matchesAcceptance;
});

// Return new group object with filtered occurrences
return {
...group,
allOccurrences: filteredOccurrences,
totalCount: filteredOccurrences.length
};
}).filter(group => group.allOccurrences.length > 0); // Remove groups with no matching occurrences

// Display filtered titles
displayTitlesCallback('commentTitles', filteredTitles, selectedPriorities, selectedAcceptanceStatus);
}

/**
* Displays distribution data in a grid
* @param {string} elementId - ID of the container element
Expand Down Expand Up @@ -248,8 +429,11 @@ export function displayDistribution(elementId, distribution) {
* @param {string} elementId - ID of the container element
* @param {Array} groups - Array of title groups
* @param {Set} selectedPriorities - Set of selected priorities (for empty state message)
* @param {string} selectedAcceptanceStatus - Selected acceptance status ('all', 'accepted', or 'not-accepted')
* @param {Object} currentData - Current PR data (optional, for toggle functionality)
* @param {Function} onAcceptanceToggle - Callback when acceptance is toggled (optional)
*/
export function displayTitles(elementId, groups, selectedPriorities) {
export function displayTitles(elementId, groups, selectedPriorities, selectedAcceptanceStatus = 'all', currentData = null, onAcceptanceToggle = null) {
const container = document.getElementById(elementId);
container.innerHTML = '';

Expand Down Expand Up @@ -303,14 +487,25 @@ export function displayTitles(elementId, groups, selectedPriorities) {
`;
itemsContainer.appendChild(itemTitleDiv);

// Add links to each occurrence
// Add links to each occurrence with acceptance toggle
item.occurrences.forEach(occurrence => {
const linkDiv = document.createElement('div');
linkDiv.className = 'title-occurrence';

const acceptedClass = occurrence.accepted ? 'accepted' : '';
const acceptedIcon = occurrence.accepted ? '✓' : '○';
const methodLabel = occurrence.accepted && occurrence.acceptanceMethod
? `(${occurrence.acceptanceMethod})`
: '';

linkDiv.innerHTML = `
<button class="acceptance-toggle ${acceptedClass}" data-url="${occurrence.url}" title="Toggle acceptance">
${acceptedIcon}
</button>
<a href="${occurrence.url}" target="_blank" class="comment-link" title="View comment on GitHub">
🔗 PR #${occurrence.prNumber}
</a>
${methodLabel ? `<span class="acceptance-method">${methodLabel}</span>` : ''}
`;
itemsContainer.appendChild(linkDiv);
});
Expand Down Expand Up @@ -344,10 +539,21 @@ export function displayTitles(elementId, groups, selectedPriorities) {
group.allOccurrences.forEach(occurrence => {
const linkDiv = document.createElement('div');
linkDiv.className = 'title-occurrence';

const acceptedClass = occurrence.accepted ? 'accepted' : '';
const acceptedIcon = occurrence.accepted ? '✓' : '○';
const methodLabel = occurrence.accepted && occurrence.acceptanceMethod
? `(${occurrence.acceptanceMethod})`
: '';

linkDiv.innerHTML = `
<button class="acceptance-toggle ${acceptedClass}" data-url="${occurrence.url}" title="Toggle acceptance">
${acceptedIcon}
</button>
<a href="${occurrence.url}" target="_blank" class="comment-link" title="View comment on GitHub">
🔗 PR #${occurrence.prNumber}
</a>
${methodLabel ? `<span class="acceptance-method">${methodLabel}</span>` : ''}
`;
occurrencesContainer.appendChild(linkDiv);
});
Expand All @@ -357,6 +563,18 @@ export function displayTitles(elementId, groups, selectedPriorities) {

container.appendChild(groupDiv);
});

// Initialize acceptance toggle buttons if currentData is provided
if (currentData && onAcceptanceToggle) {
const toggleButtons = container.querySelectorAll('.acceptance-toggle');
toggleButtons.forEach(button => {
button.addEventListener('click', async (e) => {
e.preventDefault();
const url = button.getAttribute('data-url');
await toggleManualAcceptance(url, currentData, onAcceptanceToggle);
});
});
}
Comment on lines +566 to +577

@coderabbitai coderabbitai Bot Feb 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Acceptance toggle callback signature mismatch.

The toggleManualAcceptance function expects (url, currentData, onToggle) parameters, but the event handler passes different arguments. Looking at the function signature at line 621: toggleManualAcceptance(url, currentData, onToggle), the call at line 576 passes (url, currentData, onAcceptanceToggle) which is correct.

However, the onAcceptanceToggle callback in popup.js (line 372-377) has signature (url, currentState) but toggleManualAcceptance calls onToggle() with no arguments (line 653-655). This inconsistency won't cause a crash but the return value logic in popup.js is broken.

🤖 Prompt for AI Agents 
In `@extension/filter-utils.js` around lines 568 - 579, The callback signature
mismatch: update toggleManualAcceptance to call the provided onToggle with the
expected arguments (e.g., onToggle(url, newState)) instead of calling onToggle()
with no args; locate the function named toggleManualAcceptance and change the
invocation of onToggle() (and any place that determines acceptance state) to
pass the URL and the new/current acceptance state so the onAcceptanceToggle
callback receives (url, currentState) as implemented in popup.js; keep the event
handler that passes onAcceptanceToggle unchanged.

}

/**
Expand All @@ -369,3 +587,95 @@ export function escapeHtml(text) {
div.textContent = text;
return div.innerHTML;
}

/**
* Loads manual acceptance state from storage
* @returns {Promise<Object>} Object mapping comment URLs to acceptance state
*/
export async function loadManualAcceptanceState() {
return new Promise((resolve) => {
chrome.storage.local.get(['manualAcceptance'], (result) => {
resolve(result.manualAcceptance || {});
});
});
}

/**
* Saves manual acceptance state to storage
* @param {Object} state - Object mapping comment URLs to acceptance state
*/
export async function saveManualAcceptanceState(state) {
return new Promise((resolve) => {
chrome.storage.local.set({ manualAcceptance: state }, resolve);
});
}

/**
* Toggles manual acceptance for an issue
* @param {string} url - Comment URL
* @param {Object} currentData - Current PR data
* @param {Function} onToggle - Callback when toggle completes
*/
export async function toggleManualAcceptance(url, currentData, onToggle) {
const state = await loadManualAcceptanceState();

// Toggle the state
if (state[url]) {
delete state[url];
} else {
state[url] = {
accepted: true,
acceptanceMethod: 'manual',
timestamp: new Date().toISOString()
};
}

await saveManualAcceptanceState(state);

// Update the issue in currentData
currentData.pullRequests.forEach(pr => {
pr.actionableIssues.forEach(issue => {
if (issue.url === url) {
if (state[url]) {
issue.accepted = true;
issue.acceptanceMethod = 'manual';
} else {
// Revert to automated detection state or default
issue.accepted = false;
issue.acceptanceMethod = null;
}
}
});
});

if (onToggle) {
onToggle();
}
}
Comment on lines +619 to +654

@coderabbitai coderabbitai Bot Feb 10, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

When manual acceptance is removed, original GraphQL state is lost.

When toggling off manual acceptance (lines 644-648), the code resets to accepted: false and acceptanceMethod: null instead of restoring the original GraphQL-detected state. If a comment was marked as accepted via GraphQL resolution, removing the manual override will incorrectly show it as not accepted.

🔧 Proposed fix: Store and restore original state

Consider storing the original acceptance state before applying manual override, or re-fetch/re-compute the GraphQL state when removing manual override:

   // Update the issue in currentData
   currentData.pullRequests.forEach(pr => {
     pr.actionableIssues.forEach(issue => {
       if (issue.url === url) {
         if (state[url]) {
           issue.accepted = true;
           issue.acceptanceMethod = 'manual';
         } else {
-          // Revert to automated detection state or default
-          issue.accepted = false;
-          issue.acceptanceMethod = null;
+          // Revert to original automated detection state if available
+          issue.accepted = issue.originalAccepted || false;
+          issue.acceptanceMethod = issue.originalAcceptanceMethod || null;
         }
       }
     });
   });

This requires storing originalAccepted and originalAcceptanceMethod when first applying manual state.

🤖 Prompt for AI Agents 
In `@extension/filter-utils.js` around lines 621 - 656, toggleManualAcceptance
currently overwrites the original GraphQL-detected acceptance when applying or
removing a manual override; modify it to persist the original acceptance info
before setting manual and restore that info when removing manual override: when
you set state[url] for manual acceptance (in toggleManualAcceptance and where
loadManualAcceptanceState/saveManualAcceptanceState are used) save the
originalAccepted and originalAcceptanceMethod (e.g.,
state[url].originalAccepted, state[url].originalAcceptanceMethod) populated from
the matching issue in currentData.pullRequests, and when toggling off use those
stored fields to restore issue.accepted and issue.acceptanceMethod instead of
hardcoding false/null; ensure saveManualAcceptanceState persists the original
fields and update the currentData iteration (currentData.pullRequests.forEach ->
pr.actionableIssues.forEach) to prefer state[url].original* when clearing the
manual override.


/**
* Applies manual acceptance state from storage to PR data
* Manual state has highest priority and overrides automated detection
* @param {Object} data - PR data to update
*/
export async function applyManualAcceptanceState(data) {
// Safety check: ensure data and pullRequests exist
if (!data || !data.pullRequests || !Array.isArray(data.pullRequests)) {
console.warn('applyManualAcceptanceState: Invalid data structure', data);
return;
}

const state = await loadManualAcceptanceState();

data.pullRequests.forEach(pr => {
if (pr.actionableIssues && Array.isArray(pr.actionableIssues)) {
pr.actionableIssues.forEach(issue => {
// Manual acceptance has highest priority
if (state[issue.url]) {
issue.accepted = state[issue.url].accepted;
issue.acceptanceMethod = state[issue.url].acceptanceMethod;
}
});
}
});
}
Loading