Testing - #129
Conversation
github-actions
Bot
added
no-issue-linked
|
Warning Review limit reached
Next review available in: 39 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
WalkthroughVitest scripts, dependencies, configuration, and DOM cleanup were added. New tests cover analytics behavior and number formatting. Overview statistics now use a shared formatter for compact numeric display. ChangesTesting and number formatting
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (3 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
github-actions
Bot
added
size/XL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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 `@package.json`:
- Around line 24-31: Update the vite dependency in package.json to a version
within vitest@^4.1.10’s supported range (^6.0.0, ^7.0.0, or ^8.0.0), while
leaving the existing Vitest dependency unchanged.
In `@src/services/analytics.buildTimeSeries.test.js`:
- Around line 71-82: Strengthen the assertions in the weekly case of
buildTimeSeries so the two input dates are verified to produce exactly two
buckets, preserving the existing week-format label validation. Keep the test
description aligned with this distinct-weeks behavior and remove the
tautological length check.
In `@src/services/analytics.healthMetrics.test.js`:
- Around line 75-77: Make the exact 30-day boundary test deterministic by
freezing or mocking the clock before calling daysAgoISO(30) and
computeActivityClassification, then restore the real timers after the test.
Update the test case containing daysAgoISO(30) while preserving its inclusive
'Thriving' assertion.
In `@src/utils/formatNumber.js`:
- Around line 1-6: Update formatNumber so rounding a scaled value to 1000 rolls
over to the next unit, preventing outputs such as “1000.0K” and “1000.0M”;
preserve existing formatting for other values. Add boundary-focused cases to
formatNumber.test.js covering K→M and M→B transitions.
In `@vite.config.js`:
- Around line 1-4: Remove the unused named vitest import from the configuration
imports, leaving the referenced defineConfig, react, and tailwindcss imports
unchanged.
🪄 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 Plus
Run ID: 66812e96-a23a-4212-846d-7715d0e1afbc
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
package.jsonsrc/pages/OverviewPage.jsxsrc/services/analytics.buildAnalyticalModel.test.jssrc/services/analytics.buildTimeSeries.test.jssrc/services/analytics.healthMetrics.test.jssrc/test/setup.jssrc/utils/formatNumber.jssrc/utils/formatNumber.test.jsvite.config.js
| "@testing-library/jest-dom": "^7.0.0", | ||
| "@testing-library/react": "^16.3.2", | ||
| "@testing-library/user-event": "^14.6.1", | ||
| "@vitejs/plugin-react": "^4.3.1", | ||
| "vite": "^5.4.6" | ||
| "@vitest/coverage-v8": "^4.1.10", | ||
| "jsdom": "^29.1.1", | ||
| "vite": "^5.4.6", | ||
| "vitest": "^4.1.10" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Does vitest 4.1.10 require vite 6 or higher as a peer dependency?
💡 Result:
No, Vitest 4.1.10 does not require Vite 6 (or higher) specifically as a peer dependency. Instead, Vite is listed as a standard dependency in the Vitest package, meaning it is required as a core component for Vitest to function [1][2][3]. While Vitest 4.0 and later versions require Vite >= 6.0.0 to operate [4][5], this requirement is enforced through the package's dependencies rather than as a peer dependency [2][3]. Some internal sub-packages of Vitest (such as @vitest/mocker) do utilize peer dependencies to manage their relationship with Vite [6][7], but the main vitest package itself maintains Vite as a direct dependency [1]. In summary, you must have a compatible version of Vite (Vite 6 or higher for Vitest 4.x) installed in your project for Vitest to work, but it is not technically a peer dependency of the main vitest package [1][2][4].
Citations:
- 1: https://www.npmjs.com/package/vitest?activeTab=dependencies
- 2: Should vite be a peerDependency instead of a full dependency? vitest-dev/vitest#4142
- 3: feat!: support Vite 6 vitest-dev/vitest#7026
- 4: https://vitest.dev/guide/
- 5: Migration documentation doesn't specify Vite version required. vitest-dev/vitest#9646
- 6: @vitest/mocker 4.1.0: vite declared as optional peer dependency but imported unconditionally in dist/node.js vitest-dev/vitest#9915
- 7: @vitest/mocker peerDependencies are not updated vitest-dev/vitest#9807
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package.json relevant dependency lines =="
if [ -f package.json ]; then
nl -ba package.json | sed -n '1,80p'
else
echo "package.json not found"
fi
echo
echo "== Vitest package metadata for 4.1.10 =="
if command -v npm >/dev/null 2>&1; then
npm view vitest@4.1.10 dependencies peerDependencies engines --json
fi
echo
echo "== `@vitest/coverage-v8` package metadata for 4.1.10 =="
if command -v npm >/dev/null 2>&1; then
npm view `@vitest/coverage-v8`@4.1.10 dependencies peerDependencies engines --json
fiRepository: AOSSIE-Org/OrgExplorer
Length of output: 245
🏁 Script executed:
#!/bin/sh
set -euo pipefail
echo "== package.json relevant dependency lines =="
if [ -f package.json ]; then
awk '{ printf "%5d\t%s\n", NR, $0 }' package.json | sed -n '1,80p'
else
echo "package.json not found"
fi
echo
echo "== Vitest package metadata for 4.1.10 =="
if command -v npm >/dev/null 2>&1; then
npm view vitest@4.1.10 dependencies peerDependencies engines --json
else
echo "npm not available"
fi
echo
echo "== `@vitest/coverage-v8` package metadata for 4.1.10 =="
if command -v npm >/dev/null 2>&1; then
npm view `@vitest/coverage-v8`@4.1.10 dependencies peerDependencies engines --json
else
echo "npm not available"
fiRepository: AOSSIE-Org/OrgExplorer
Length of output: 3531
Align Vite with Vitest’s supported range.
vitest@4.1.10 declares Vite ^6.0.0 || ^7.0.0 || ^8.0.0, so vite@^5.4.6 leaves the test suite with incompatible Vite versions / unresolved Vitest runtime dependencies.
🔧 Suggested fix
- "vite": "^5.4.6",
+ "vite": "^6.0.0",📝 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.
| "@testing-library/jest-dom": "^7.0.0", | |
| "@testing-library/react": "^16.3.2", | |
| "@testing-library/user-event": "^14.6.1", | |
| "@vitejs/plugin-react": "^4.3.1", | |
| "vite": "^5.4.6" | |
| "@vitest/coverage-v8": "^4.1.10", | |
| "jsdom": "^29.1.1", | |
| "vite": "^5.4.6", | |
| "vitest": "^4.1.10" | |
| "`@testing-library/jest-dom`": "^7.0.0", | |
| "`@testing-library/react`": "^16.3.2", | |
| "`@testing-library/user-event`": "^14.6.1", | |
| "`@vitejs/plugin-react`": "^4.3.1", | |
| "`@vitest/coverage-v8`": "^4.1.10", | |
| "jsdom": "^29.1.1", | |
| "vite": "^6.0.0", | |
| "vitest": "^4.1.10" |
🤖 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 `@package.json` around lines 24 - 31, Update the vite dependency in
package.json to a version within vitest@^4.1.10’s supported range (^6.0.0,
^7.0.0, or ^8.0.0), while leaving the existing Vitest dependency unchanged.
| it('produces one bucket per week when granularity is weekly, distinct from monthly', () => { | ||
| const issues = [ | ||
| { created_at: '2026-01-01T00:00:00Z' }, | ||
| { created_at: '2026-01-08T00:00:00Z' }, | ||
| ] | ||
|
|
||
| const weekly = buildTimeSeries(issues, 'weekly') | ||
|
|
||
| // Two different weeks -> two buckets, and week-format labels | ||
| expect(weekly.length).toBeGreaterThanOrEqual(1) | ||
| weekly.forEach(bucket => expect(bucket.date).toMatch(/^\d{4}-W\d{2}$/)) | ||
| }) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
Weak assertion doesn't match the test's stated intent.
The comment says two distinct weeks should produce two buckets, but the assertion only checks length >= 1, which passes even if the function collapsed both dates into a single bucket. Tighten it to actually verify the claimed behavior.
✅ Suggested fix
const weekly = buildTimeSeries(issues, 'weekly')
- // Two different weeks -> two buckets, and week-format labels
- expect(weekly.length).toBeGreaterThanOrEqual(1)
+ // Two different weeks -> two buckets, and week-format labels
+ expect(weekly).toHaveLength(2)
weekly.forEach(bucket => expect(bucket.date).toMatch(/^\d{4}-W\d{2}$/))As per path instructions, test files should avoid tautological assertions and have descriptions that clarify the test's purpose — here the assertion should actually validate what the description promises.
📝 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.
| it('produces one bucket per week when granularity is weekly, distinct from monthly', () => { | |
| const issues = [ | |
| { created_at: '2026-01-01T00:00:00Z' }, | |
| { created_at: '2026-01-08T00:00:00Z' }, | |
| ] | |
| const weekly = buildTimeSeries(issues, 'weekly') | |
| // Two different weeks -> two buckets, and week-format labels | |
| expect(weekly.length).toBeGreaterThanOrEqual(1) | |
| weekly.forEach(bucket => expect(bucket.date).toMatch(/^\d{4}-W\d{2}$/)) | |
| }) | |
| it('produces one bucket per week when granularity is weekly, distinct from monthly', () => { | |
| const issues = [ | |
| { created_at: '2026-01-01T00:00:00Z' }, | |
| { created_at: '2026-01-08T00:00:00Z' }, | |
| ] | |
| const weekly = buildTimeSeries(issues, 'weekly') | |
| // Two different weeks -> two buckets, and week-format labels | |
| expect(weekly).toHaveLength(2) | |
| weekly.forEach(bucket => expect(bucket.date).toMatch(/^\d{4}-W\d{2}$/)) | |
| }) |
🤖 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/services/analytics.buildTimeSeries.test.js` around lines 71 - 82,
Strengthen the assertions in the weekly case of buildTimeSeries so the two input
dates are verified to produce exactly two buckets, preserving the existing
week-format label validation. Keep the test description aligned with this
distinct-weeks behavior and remove the tautological length check.
Source: Path instructions
| it('treats the exact 30-day boundary as Thriving (inclusive)', () => { | ||
| expect(computeActivityClassification({ pushed_at: daysAgoISO(30) })).toBe('Thriving') | ||
| }) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Boundary test relies on the real clock and can be flaky.
daysAgoISO(30) captures Date.now() at one instant; computeActivityClassification re-reads Date.now() when the assertion runs. Any elapsed time beyond the same millisecond pushes the computed days just over 30, causing the implementation to return 'Active' instead of the asserted 'Thriving'. Under CI load or slower runs, this can intermittently fail.
🕒 Suggested fix using fake timers
- it('treats the exact 30-day boundary as Thriving (inclusive)', () => {
- expect(computeActivityClassification({ pushed_at: daysAgoISO(30) })).toBe('Thriving')
- })
+ it('treats the exact 30-day boundary as Thriving (inclusive)', () => {
+ vi.useFakeTimers()
+ const now = Date.now()
+ const pushedAt = new Date(now - 30 * 86_400_000).toISOString()
+ vi.setSystemTime(now)
+ expect(computeActivityClassification({ pushed_at: pushedAt })).toBe('Thriving')
+ vi.useRealTimers()
+ })📝 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.
| it('treats the exact 30-day boundary as Thriving (inclusive)', () => { | |
| expect(computeActivityClassification({ pushed_at: daysAgoISO(30) })).toBe('Thriving') | |
| }) | |
| it('treats the exact 30-day boundary as Thriving (inclusive)', () => { | |
| vi.useFakeTimers() | |
| const now = Date.now() | |
| const pushedAt = new Date(now - 30 * 86_400_000).toISOString() | |
| vi.setSystemTime(now) | |
| expect(computeActivityClassification({ pushed_at: pushedAt })).toBe('Thriving') | |
| vi.useRealTimers() | |
| }) |
🤖 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/services/analytics.healthMetrics.test.js` around lines 75 - 77, Make the
exact 30-day boundary test deterministic by freezing or mocking the clock before
calling daysAgoISO(30) and computeActivityClassification, then restore the real
timers after the test. Update the test case containing daysAgoISO(30) while
preserving its inclusive 'Thriving' assertion.
| import { defineConfig } from "vitest/config"; | ||
| import react from "@vitejs/plugin-react"; | ||
| import tailwindcss from "@tailwindcss/vite"; | ||
| import { vitest } from "vitest"; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
🧩 Analysis chain
🌐 Web query:
Does the "vitest" npm package export a named binding called vitest?
💡 Result:
Yes, the vitest npm package exports a named binding called vitest [1][2][3]. This named export is available from the main entry point of the vitest package and is primarily used to access Vitest's utility functions (similar to the vi helper) [1][2][3]. You can import it using standard ESM or CommonJS syntax: import { vitest } from 'vitest'; The vitest export is maintained in the package's public API alongside other common exports like vi, expect, and various test-running functions [1][2][3].
Citations:
- 1: https://github.com/vitest-dev/vitest/blob/fda52af9c3974d5fd16161e1ed502dd47fd38ceb/packages/vitest/src/public/index.ts
- 2: https://cdn.jsdelivr.net/npm/vitest@4.0.18/dist/index.d.ts
- 3: https://tangled.org/vitest.dev/vitest/blob/ca041f51ad2395dd91d18c33b642fb346c6bfd15/packages/vitest/src/public/index.ts
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== vite.config.js =="
if [ -f vite.config.js ]; then
cat -n vite.config.js
else
echo "vite.config.js not found"
fi
echo
echo "== package versions and lockfile vitest entries =="
for f in package.json pnpm-lock.yaml package-lock.json yarn.lock bun.lockb; do
if [ -f "$f" ]; then
echo "-- $f --"
case "$f" in
package.json|package-lock.json)
node -e 'const p=require("./package.json"); console.log("package dev deps:", (p.devDependencies&&p.devDependencies.vitest||"?")+"; package deps:", p.dependencies&&p.dependencies.vitest||"?");' 2>/dev/null || true
;;
pnpm-lock.yaml) rg -n "vitest(@|:)|/vitest@" pnpm-lock.yaml | head -20 || true
;;
yarn.lock|bun.lockb)
rg -n "vitest@" "$f" 2>/dev/null | head -20 || true || true
;;
esac
fi
done
echo
echo "== usages of vitest/vitest() in repo =="
rg -n '\bvitest\b|\.{vitest}\(' --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' . || true
echo
echo "== local node_modules vitest package files if present =="
if [ -f node_modules/vitest/package.json ]; then
node -e 'const p=require("./node_modules/vitest/package.json"); console.log("vitest version", p.version); const pub={}; for (const k of ["exports","main","module","types","typesVersions"]) if (p[k]) pub[k]=p[k]; console.log(JSON.stringify(pub,null,2));'
rg -n "export \{ vitest\b|\"vitest\"" node_modules/vitest node_modules/vitest/config 2>/dev/null || true
fiRepository: AOSSIE-Org/OrgExplorer
Length of output: 1056
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package-lock.json vitest package metadata =="
python3 - <<'PY'
import json
from pathlib import Path
p=Path("package-lock.json")
data=json.loads(p.read_text())
for key, info in data.get("packages", {}).items():
if key.endswith("node_modules/vitest") or key == "node_modules/vitest":
print(key, info.get("version","?"))
for field in ("version","dependencies","devDependencies"):
print(field, info.get(field))
PY
echo
echo "== usages of literal 'vitest' and vitest() =="
rg -n --word-regexp '\bvitest\b|\bvitest\(' --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' . || true
echo
echo "== fetch vitest 4.1.10 exports from npm registry =="
python3 - <<'PY'
import json, urllib.request
url="https://registry.npmjs.org/vitest/4.1.10"
try:
data=json.loads(urllib.request.urlopen(url, timeout=20).read())
except Exception as e:
print("registry fetch failed", type(e).__name__, e)
raise SystemExit
print("version", data.get("version"))
print("exports:")
for root, val in data.get("exports", {}).items():
if isinstance(val, str):
if root in (".","./*","./config","./config/*") or "vitest" in root:
print(root, val)
elif isinstance(val, dict):
for k,v in val.items():
if k in ("default","require","import") or "vitest" in k:
print(k, v)
PYRepository: AOSSIE-Org/OrgExplorer
Length of output: 8355
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== fetch vitest package files with HTTPS_SKIP_VERIFY =="
python3 - <<'PY'
import ssl, tarfile, urllib.request, io, sys, os, json
context = ssl.create_default_context()
context.check_hostname = False
context.verify_mode = ssl.CERT_NONE
url = "https://registry.npmjs.org/vitest/4.1.10"
data = json.loads(urllib.request.urlopen(url, context=context, timeout=20).read())
tgz = data["dist"]["tarball"]
tar = tarfile.open(fileobj=urllib.request.urlopen(tgz, context=context, timeout=30), mode="r:gz")
for member in tar.getmembers():
if not member.isfile():
continue
name = member.name.split("/", 1)[-1]
if name in {"dist/package.json", "dist/index.d.ts", "dist/index.d.cts", "package.json"} and len(name) < len(member.name):
f = tar.extractfile(member)
text = f.read().decode("utf-8", "replace")
print(f"--- {member.name} ---")
for i, line in enumerate(text.splitlines(), 1):
if 1 <= i <= 320 and ("vitest" in line or "exports" in line or "vitest.mjs" in line):
print(f"{i}: {line}")
break
else:
print("No matching dist files found")
PY
echo
echo "== local package-lock vitest dist package fields =="
python3 - <<'PY'
import json
data=json.loads(open("package-lock.json", encoding="utf-8").read())
node=data["packages"].get("node_modules/vitest")
if not node:
print("missing node_modules/vitest")
else:
for k in ("dist","exports","files","main","module","type"):
print(k, json.dumps(node.get(k), indent=2))
PYRepository: AOSSIE-Org/OrgExplorer
Length of output: 881
Unused import { vitest } from "vitest".
vitest is a valid named export in Vitest, but this import is not referenced by the config, so keep it only if it’s intentionally needed elsewhere; otherwise remove it to avoid importing dead code.
🤖 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 `@vite.config.js` around lines 1 - 4, Remove the unused named vitest import
from the configuration imports, leaving the referenced defineConfig, react, and
tailwindcss imports unchanged.
2 participants
Addressed Issues:
Fixes #(issue number)
Screenshots/Recordings:
Additional Notes:
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
Tests