Added new column of stale issue ratio in Governance page and also add…#95
Conversation
github-actions
Bot
added
no-issue-linked
WalkthroughAdds a ChangesStale Issues Ratio
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 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 |
github-actions
Bot
added
size/L
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/context/AppContext.jsx`:
- Around line 179-184: The Ctx.Provider value in AppProvider is rebuilt on every
render, which forces all consumers such as GovernancePage to re-render
unnecessarily. Memoize the provider value object with useMemo and include all
referenced fields and callbacks used in the value prop. Keep the memoized object
near the existing staleRepoStats memoization so the context only changes when
one of its dependencies changes.
In `@src/pages/GovernancePage.jsx`:
- Around line 75-76: The “Stale Issues Ratio” card is using the dead-issues
metric instead of the stale-issue metric, so update the calculation in
GovernancePage to derive from staleRepoStats rather than allIssues/deadIssues.
Use the existing staleRepoStats data from AppContext to compute an aggregate
stale ratio (for example, total staleCount over total openCount), and keep
getStatus(staleIssuesRatio) pointing at that corrected value so the card matches
the “Stale Issues” tab.
- Around line 216-218: The badge color logic in GovernancePage is using a single
`counts[t.key] > 40` check for all tabs, but only the new percentage-based stale
count should use that threshold. Update the span styling logic around the counts
render so `stale` keeps the `> 40` red cutoff, while raw-count tabs like `dead`,
`zombie`, and `license` preserve their prior behavior of turning red whenever
the count is greater than 0.
- Line 54: The GovernancePage component still contains a leftover debug
console.log in the stale repos pagination flow. Remove the console.log from
GovernancePage.jsx so the page no longer emits paginatedStaleRepos to the
console, keeping the render logic and pagination behavior unchanged.
- Around line 313-339: Add explicit type="button" to the pagination controls in
GovernancePage so they do not default to submit behavior. Update the two button
elements that use setStalePage for “← Previous” and “Next →” to include the
button type attribute, keeping their current disabled logic and styling
unchanged.
- Around line 46-53: The pagination state in GovernancePage.jsx is not being
clamped when staleRepoStats changes, so stalePage can stay greater than
totalPages and show an empty view. Update the GovernancePage component to reset
or clamp stalePage whenever staleRepoStats shrinks, using the stalePage state,
totalPages calculation, and the paginatedStaleRepos useMemo as the key places to
adjust the behavior so the current page always stays within bounds.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6468ba52-3a67-49e9-834d-47824f334eb3
📒 Files selected for processing (2)
src/context/AppContext.jsxsrc/pages/GovernancePage.jsx
| return ( | ||
| <Ctx.Provider value={{ | ||
| pat, savePat, orgs, model, issuesData, | ||
| rateLimit, loading, loadMsg, govLoading, error, totalRepo, | ||
| explore, runAudit, setError, | ||
| explore, runAudit, setError, staleRepoStats, | ||
| }}> |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🔵 Trivial | ⚖️ Poor tradeoff
Memoize the context value to avoid re-rendering every consumer.
The Ctx.Provider value object is reconstructed on every render of AppProvider, so every consumer (e.g. GovernancePage) re-renders even when unrelated state changes. Since useMemo is already imported for staleRepoStats, wrapping the provider value is a low-effort follow-up.
♻️ Suggested fix
+ const ctxValue = useMemo(() => ({
+ pat, savePat, orgs, model, issuesData,
+ rateLimit, loading, loadMsg, govLoading, error, totalRepo,
+ explore, runAudit, setError, staleRepoStats,
+ }), [pat, savePat, orgs, model, issuesData, rateLimit, loading, loadMsg, govLoading, error, totalRepo, explore, runAudit, staleRepoStats])
+
return (
- <Ctx.Provider value={{
- pat, savePat, orgs, model, issuesData,
- rateLimit, loading, loadMsg, govLoading, error, totalRepo,
- explore, runAudit, setError, staleRepoStats,
- }}>
+ <Ctx.Provider value={ctxValue}>
{children}
</Ctx.Provider>
)📝 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.
| return ( | |
| <Ctx.Provider value={{ | |
| pat, savePat, orgs, model, issuesData, | |
| rateLimit, loading, loadMsg, govLoading, error, totalRepo, | |
| explore, runAudit, setError, | |
| explore, runAudit, setError, staleRepoStats, | |
| }}> | |
| const ctxValue = useMemo(() => ({ | |
| pat, savePat, orgs, model, issuesData, | |
| rateLimit, loading, loadMsg, govLoading, error, totalRepo, | |
| explore, runAudit, setError, staleRepoStats, | |
| }), [pat, savePat, orgs, model, issuesData, rateLimit, loading, loadMsg, govLoading, error, totalRepo, explore, runAudit, staleRepoStats]) | |
| return ( | |
| <Ctx.Provider value={ctxValue}> | |
| {children} | |
| </Ctx.Provider> | |
| ) |
🧰 Tools
🪛 React Doctor (0.5.8)
[warning] 180-180: Every reader of this context redraws on each render because you build its value inline.
Wrap the context value in useMemo or move it outside the component so consumers do not redraw every render.
(jsx-no-constructed-context-values)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/context/AppContext.jsx` around lines 179 - 184, The Ctx.Provider value in
AppProvider is rebuilt on every render, which forces all consumers such as
GovernancePage to re-render unnecessarily. Memoize the provider value object
with useMemo and include all referenced fields and callbacks used in the value
prop. Keep the memoized object near the existing staleRepoStats memoization so
the context only changes when one of its dependencies changes.
Source: Linters/SAST tools
| const ITEMS_PER_PAGE = 10 | ||
| const [stalePage, setStalePage] = useState(1) | ||
| const totalPages = Math.ceil(staleRepoStats.length / ITEMS_PER_PAGE) | ||
|
|
||
| const paginatedStaleRepos = useMemo(() => { | ||
| const start = (stalePage - 1) * ITEMS_PER_PAGE | ||
| return staleRepoStats.slice(start, start + ITEMS_PER_PAGE) | ||
| }, [staleRepoStats, stalePage]) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
stalePage isn't reset when staleRepoStats shrinks.
If a re-run audit yields fewer repos, stalePage can remain past totalPages, showing an empty page with "Page X of Y" where X > Y (Next stays enabled since stalePage === totalPages never holds).
🐛 Suggested fix
+ useEffect(() => {
+ setStalePage(1)
+ }, [staleRepoStats])📝 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 ITEMS_PER_PAGE = 10 | |
| const [stalePage, setStalePage] = useState(1) | |
| const totalPages = Math.ceil(staleRepoStats.length / ITEMS_PER_PAGE) | |
| const paginatedStaleRepos = useMemo(() => { | |
| const start = (stalePage - 1) * ITEMS_PER_PAGE | |
| return staleRepoStats.slice(start, start + ITEMS_PER_PAGE) | |
| }, [staleRepoStats, stalePage]) | |
| const ITEMS_PER_PAGE = 10 | |
| const [stalePage, setStalePage] = useState(1) | |
| const totalPages = Math.ceil(staleRepoStats.length / ITEMS_PER_PAGE) | |
| useEffect(() => { | |
| setStalePage(1) | |
| }, [staleRepoStats]) | |
| const paginatedStaleRepos = useMemo(() => { | |
| const start = (stalePage - 1) * ITEMS_PER_PAGE | |
| return staleRepoStats.slice(start, start + ITEMS_PER_PAGE) | |
| }, [staleRepoStats, stalePage]) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/GovernancePage.jsx` around lines 46 - 53, The pagination state in
GovernancePage.jsx is not being clamped when staleRepoStats changes, so
stalePage can stay greater than totalPages and show an empty view. Update the
GovernancePage component to reset or clamp stalePage whenever staleRepoStats
shrinks, using the stalePage state, totalPages calculation, and the
paginatedStaleRepos useMemo as the key places to adjust the behavior so the
current page always stays within bounds.
| const start = (stalePage - 1) * ITEMS_PER_PAGE | ||
| return staleRepoStats.slice(start, start + ITEMS_PER_PAGE) | ||
| }, [staleRepoStats, stalePage]) | ||
| console.log(paginatedStaleRepos); |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Remove leftover debug console.log.
🧹 Suggested fix
- console.log(paginatedStaleRepos);📝 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.
| console.log(paginatedStaleRepos); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/GovernancePage.jsx` at line 54, The GovernancePage component still
contains a leftover debug console.log in the stale repos pagination flow. Remove
the console.log from GovernancePage.jsx so the page no longer emits
paginatedStaleRepos to the console, keeping the render logic and pagination
behavior unchanged.
| // Health check 2 — Percentage of dead issues relative to all issues | ||
| const staleIssuesRatio = allIssues.length ? (deadIssues.length / allIssues.length) * 100 : 0; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
"Stale Issues Ratio" stat card shows the wrong metric.
staleIssuesRatio is computed as dead issues (created_at ≥ 90 days) divided by all issues — the same definition used for the "Dead Issues" health check, not the new stale-issue metric (updated_at inactivity) computed as staleRepoStats in AppContext. The stat card labeled "Stale Issues Ratio" and its severity color (getStatus(staleIssuesRatio)) will therefore show numbers inconsistent with the "Stale Issues" tab built from staleRepoStats, which is confusing since both are presented under the same "stale" concept on this page.
Consider deriving the stat card from staleRepoStats instead, e.g. an aggregate ratio across all repos:
const totalStale = staleRepoStats.reduce((s, r) => s + r.staleCount, 0)
const totalOpen = staleRepoStats.reduce((s, r) => s + r.openCount, 0)
const staleIssuesRatio = totalOpen ? (totalStale / totalOpen) * 100 : 0Also applies to: 158-158
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/GovernancePage.jsx` around lines 75 - 76, The “Stale Issues Ratio”
card is using the dead-issues metric instead of the stale-issue metric, so
update the calculation in GovernancePage to derive from staleRepoStats rather
than allIssues/deadIssues. Use the existing staleRepoStats data from AppContext
to compute an aggregate stale ratio (for example, total staleCount over total
openCount), and keep getStatus(staleIssuesRatio) pointing at that corrected
value so the card matches the “Stale Issues” tab.
| <span style={{ color: counts[t.key] > 40 ? 'var(--red)' : 'var(--green)', marginLeft: 4 }}> | ||
| {counts[t.key]} | ||
| </span> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Badge threshold > 40 is misapplied to raw-count tabs.
This condition now drives the badge color for dead, zombie, and license tabs too, but those counts values are raw counts (e.g. deadIssues.length, which per the PR screenshots can be in the thousands), not percentages. A threshold of 40 makes sense for the new percentage-based stale count but silently changes when dead/zombie/license badges turn red (previously any count > 0).
🐛 Suggested fix
- <span style={{ color: counts[t.key] > 40 ? 'var(--red)' : 'var(--green)', marginLeft: 4 }}>
+ <span style={{ color: (t.key === 'stale' ? counts[t.key] > 40 : counts[t.key] > 0) ? 'var(--red)' : 'var(--green)', marginLeft: 4 }}>📝 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.
| <span style={{ color: counts[t.key] > 40 ? 'var(--red)' : 'var(--green)', marginLeft: 4 }}> | |
| {counts[t.key]} | |
| </span> | |
| <span style={{ color: (t.key === 'stale' ? counts[t.key] > 40 : counts[t.key] > 0) ? 'var(--red)' : 'var(--green)', marginLeft: 4 }}> | |
| {counts[t.key]} | |
| </span> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/GovernancePage.jsx` around lines 216 - 218, The badge color logic
in GovernancePage is using a single `counts[t.key] > 40` check for all tabs, but
only the new percentage-based stale count should use that threshold. Update the
span styling logic around the counts render so `stale` keeps the `> 40` red
cutoff, while raw-count tabs like `dead`, `zombie`, and `license` preserve their
prior behavior of turning red whenever the count is greater than 0.
| <button | ||
| onClick={() => setStalePage(p => p - 1)} | ||
| disabled={stalePage === 1} | ||
| style={{ | ||
| padding: '8px 14px', | ||
| cursor: stalePage === 1 ? 'not-allowed' : 'pointer', | ||
| opacity: stalePage === 1 ? 0.5 : 1 | ||
| }} | ||
| > | ||
| ← Previous | ||
| </button> | ||
|
|
||
| <span style={{ fontSize: 13, color: 'var(--text2)' }}> | ||
| Page {stalePage} of {totalPages} | ||
| </span> | ||
|
|
||
| <button | ||
| onClick={() => setStalePage(p => p + 1)} | ||
| disabled={stalePage === totalPages} | ||
| style={{ | ||
| padding: '8px 14px', | ||
| cursor: stalePage === totalPages ? 'not-allowed' : 'pointer', | ||
| opacity: stalePage === totalPages ? 0.5 : 1 | ||
| }} | ||
| > | ||
| Next → | ||
| </button> |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Add explicit type="button" to pagination controls.
Without an explicit type, these buttons default to type="submit" and could trigger accidental form submission if this markup is ever nested in a <form>.
🔧 Suggested fix
<button
+ type="button"
onClick={() => setStalePage(p => p - 1)}
disabled={stalePage === 1} <button
+ type="button"
onClick={() => setStalePage(p => p + 1)}
disabled={stalePage === totalPages}📝 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.
| <button | |
| onClick={() => setStalePage(p => p - 1)} | |
| disabled={stalePage === 1} | |
| style={{ | |
| padding: '8px 14px', | |
| cursor: stalePage === 1 ? 'not-allowed' : 'pointer', | |
| opacity: stalePage === 1 ? 0.5 : 1 | |
| }} | |
| > | |
| ← Previous | |
| </button> | |
| <span style={{ fontSize: 13, color: 'var(--text2)' }}> | |
| Page {stalePage} of {totalPages} | |
| </span> | |
| <button | |
| onClick={() => setStalePage(p => p + 1)} | |
| disabled={stalePage === totalPages} | |
| style={{ | |
| padding: '8px 14px', | |
| cursor: stalePage === totalPages ? 'not-allowed' : 'pointer', | |
| opacity: stalePage === totalPages ? 0.5 : 1 | |
| }} | |
| > | |
| Next → | |
| </button> | |
| <button | |
| type="button" | |
| onClick={() => setStalePage(p => p - 1)} | |
| disabled={stalePage === 1} | |
| style={{ | |
| padding: '8px 14px', | |
| cursor: stalePage === 1 ? 'not-allowed' : 'pointer', | |
| opacity: stalePage === 1 ? 0.5 : 1 | |
| }} | |
| > | |
| ← Previous | |
| </button> | |
| <span style={{ fontSize: 13, color: 'var(--text2)' }}> | |
| Page {stalePage} of {totalPages} | |
| </span> | |
| <button | |
| type="button" | |
| onClick={() => setStalePage(p => p + 1)} | |
| disabled={stalePage === totalPages} | |
| style={{ | |
| padding: '8px 14px', | |
| cursor: stalePage === totalPages ? 'not-allowed' : 'pointer', | |
| opacity: stalePage === totalPages ? 0.5 : 1 | |
| }} | |
| > | |
| Next → | |
| </button> |
🧰 Tools
🪛 React Doctor (0.5.8)
[warning] 313-313: Your users can submit the form by accident because a <button> with no type defaults to submit.
Set an explicit button type so plain buttons do not submit forms by accident: type="button", "submit", or "reset".
(button-has-type)
[warning] 329-329: Your users can submit the form by accident because a <button> with no type defaults to submit.
Set an explicit button type so plain buttons do not submit forms by accident: type="button", "submit", or "reset".
(button-has-type)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/GovernancePage.jsx` around lines 313 - 339, Add explicit
type="button" to the pagination controls in GovernancePage so they do not
default to submit behavior. Update the two button elements that use setStalePage
for “← Previous” and “Next →” to include the button type attribute, keeping
their current disabled logic and styling unchanged.
Source: Linters/SAST tools
2 participants
Screenshots/Recordings:
Before: -
After: -
https://1drv.ms/v/c/2b55451e453f4443/IQDU_RROSmoWTIQVDSiI5d1VASodqthNAW73a3yuu1QO_is?e=D9CcI3
Additional Notes:
Stale issue count
Open issue count
Stale issue ratio
Health status (Excellent, Healthy, Warning, Critical)
This helps users quickly identify repositories with inactive issues and navigate directly to GitHub for further investigation.
Checklist
We encourage contributors to use AI tools responsibly when creating Pull Requests. While AI can be a valuable aid, it is essential to ensure that your contributions meet the task requirements, build successfully, include relevant tests, and pass all linters. Submissions that do not meet these standards may be closed without warning to maintain the quality and integrity of the project. Please take the time to understand the changes you are proposing and their impact.
Summary by CodeRabbit
New Features
Bug Fixes