Add CodSpeed performance benchmarks#51
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Review changes with Changed Files
|
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
✅ Deploy Preview for lsngames ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
View changes in DiffLens |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
|
View changes in DiffLens |
Reviewer's GuideIntegrates CodSpeed-backed Vitest benchmarks by extracting translation and game catalog logic into reusable lib modules, adding a Vitest+CodSpeed benchmark suite, wiring a CodSpeed CI workflow, and updating the app and README to use the new utilities and badge. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![codereviewbot-ai[bot]](https://avatars.githubusercontent.com/in/309638?s=60&v=4){kind=small}
| export default function Home() { | ||
| const [lang, setLang] = useState<Language>("en"); | ||
| const t = translations[lang]; | ||
| const t = getTranslation(lang); |
There was a problem hiding this comment.
Potential Error Handling Issue:
The getTranslation(lang) function is called directly without any error handling or validation. If getTranslation can throw an error or return an undefined/null value (e.g., for an unsupported language), this could lead to runtime errors or broken UI.
Recommendation:
Add error handling or validation to ensure t is always a valid translation object. For example:
const t = getTranslation(lang) || { title: 'Coming Soon', subtitle: '' };Or wrap in a try/catch if exceptions are possible.
| import { getTranslation, type Language } from "@/lib/translations"; | ||
|
|
||
| export default function Home() { | ||
| const [lang, setLang] = useState<Language>("en"); |
There was a problem hiding this comment.
Type Safety Concern:
The setLang function is called with e.target.value as Language, which relies on a type assertion. If the select input is ever extended or manipulated, this could allow invalid values to be set, potentially causing issues in translation lookup.
Recommendation:
Validate the selected value before updating the state, or restrict the select options to only valid Language values. For example:
const validLanguages: Language[] = ['en', 'zh'];
const selectedLang = e.target.value;
if (validLanguages.includes(selectedLang as Language)) {
setLang(selectedLang as Language);
}This ensures only valid languages are set.
| describe("translations", () => { | ||
| bench("getTranslation - existing language", () => { | ||
| getTranslation("en"); | ||
| }); | ||
|
|
||
| bench("getTranslation - fallback to default", () => { | ||
| getTranslation("fr"); | ||
| }); | ||
|
|
||
| bench("getAvailableLanguages", () => { | ||
| getAvailableLanguages(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("game catalog - small (50 games)", () => { | ||
| bench("filterGamesByCategory", () => { | ||
| filterGamesByCategory(smallCatalog, "Action"); | ||
| }); | ||
|
|
||
| bench("sortGamesByTitle", () => { | ||
| sortGamesByTitle(smallCatalog); | ||
| }); | ||
|
|
||
| bench("searchGames", () => { | ||
| searchGames(smallCatalog, "unique mechanics"); | ||
| }); | ||
|
|
||
| bench("getUniqueCategories", () => { | ||
| getUniqueCategories(smallCatalog); | ||
| }); | ||
|
|
||
| bench("paginateGames", () => { | ||
| paginateGames(smallCatalog, 2, 10); | ||
| }); | ||
| }); | ||
|
|
||
| describe("game catalog - large (1000 games)", () => { | ||
| bench("filterGamesByCategory", () => { | ||
| filterGamesByCategory(largeCatalog, "Strategy"); | ||
| }); | ||
|
|
||
| bench("sortGamesByTitle", () => { | ||
| sortGamesByTitle(largeCatalog); | ||
| }); | ||
|
|
||
| bench("searchGames", () => { | ||
| searchGames(largeCatalog, "unique mechanics"); | ||
| }); | ||
|
|
||
| bench("getUniqueCategories", () => { | ||
| getUniqueCategories(largeCatalog); | ||
| }); | ||
|
|
||
| bench("paginateGames", () => { | ||
| paginateGames(largeCatalog, 5, 20); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The benchmarks currently do not validate the correctness of the results returned by the functions under test. This means that if a function's implementation changes and produces incorrect results, the benchmarks will not detect it. Consider adding assertions to verify that the output is as expected, even in performance tests. For example:
bench("filterGamesByCategory", () => {
const result = filterGamesByCategory(smallCatalog, "Action");
if (!Array.isArray(result) || result.some(g => g.category !== "Action")) {
throw new Error("filterGamesByCategory returned incorrect results");
}
});This ensures that performance measurements are only taken for correct implementations.
| export function paginateGames( | ||
| games: Game[], | ||
| page: number, | ||
| pageSize: number | ||
| ): Game[] { | ||
| const start = (page - 1) * pageSize; | ||
| return games.slice(start, start + pageSize); | ||
| } |
There was a problem hiding this comment.
Lack of input validation in paginateGames
The paginateGames function does not validate the page and pageSize parameters. If page is less than 1 or pageSize is less than 1, the function may return an empty array or unexpected results, which could lead to bugs or inconsistent behavior in the application.
Recommended solution:
Add input validation to ensure page and pageSize are positive integers:
if (page < 1 || pageSize < 1) {
throw new Error('Page and pageSize must be positive integers');
}| export function getTranslation(lang: string) { | ||
| return translations[lang] ?? translations["en"]; | ||
| } |
There was a problem hiding this comment.
Logic & Type Safety:
The getTranslation function accepts any string as input, but only known language keys are valid. This can lead to unexpected behavior if an invalid language code is passed. Consider restricting the input type to Language or validating the input before accessing the translations object:
export function getTranslation(lang: Language) {
return translations[lang];
}Or, if you must accept any string, validate it:
export function getTranslation(lang: string) {
if (lang in translations) {
return translations[lang];
}
return translations["en"];
}| export function getAvailableLanguages(): string[] { | ||
| return Object.keys(translations); | ||
| } |
There was a problem hiding this comment.
Type Consistency:
The getAvailableLanguages function returns string[], but the actual set of keys is defined by the Language type. For improved type safety and consistency, update the return type to Language[]:
export function getAvailableLanguages(): Language[] {
return Object.keys(translations) as Language[];
}
|
View changes in DiffLens |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Congrats! CodSpeed is installed 🎉
You will start to see performance impacts in the reports once the benchmarks are run from your default branch.
|
❌ 2 blocking issues (4 total)
|
![qltysh[bot]](https://avatars.githubusercontent.com/in/890766?s=60&v=4){kind=small}
| name: Run benchmarks | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/checkout@v4 |
| run: npm ci | ||
|
|
||
| - name: Run benchmarks | ||
| uses: CodSpeedHQ/action@v4 |
|
|
||
| bench("paginateGames", () => { | ||
| paginateGames(smallCatalog, 2, 10); | ||
| }); |
|
|
||
| bench("paginateGames", () => { | ||
| paginateGames(largeCatalog, 5, 20); | ||
| }); |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Compatibility | 1 medium 4 high |
| BestPractice | 13 medium 2 minor 4 high |
| Documentation | 1 minor |
| ErrorProne | 6 high |
| Security | 5 high |
| CodeStyle | 43 minor |
| Complexity | 1 medium |
🟢 Metrics 33 complexity · 0 duplication
Metric Results Complexity 33 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- The
translationsutilities could be made more type-safe by narrowing the key type (e.g.Record<Language, ...>andas const) and updatinggetTranslation/getAvailableLanguagesto useLanguageinstead ofstring, so callers get compile-time checks on language codes. getTranslationcurrently accepts astringand indexestranslationswithout validation, which can yieldundefinedat the type level; consider tightening its signature and return type (e.g.getTranslation(lang: Language | string): Translation) to reflect the fallback behavior and avoid implicitany/undefined.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `translations` utilities could be made more type-safe by narrowing the key type (e.g. `Record<Language, ...>` and `as const`) and updating `getTranslation`/`getAvailableLanguages` to use `Language` instead of `string`, so callers get compile-time checks on language codes.
- `getTranslation` currently accepts a `string` and indexes `translations` without validation, which can yield `undefined` at the type level; consider tightening its signature and return type (e.g. `getTranslation(lang: Language | string): Translation`) to reflect the fallback behavior and avoid implicit `any`/`undefined`.
## Individual Comments
### Comment 1
<location path="lib/translations.ts" line_range="1-2" />
<code_context>
+export const translations: Record<
+ string,
+ { title: string; subtitle: string; language: string }
+> = {
</code_context>
<issue_to_address>
**issue:** The explicit `Record<string, ...>` annotation widens `Language` to `string` and loses the concrete language union.
Because `translations` is explicitly typed as `Record<string, { ... }>`, `keyof typeof translations` is just `string`, so `Language` is widened to `string` instead of the intended union of concrete language codes. This removes useful type checking and allows any string to be passed where a known language is expected.
You can keep the literal key union by relying on inference and `as const satisfies`, for example:
```ts
export const translations = {
en: { ... },
zh: { ... },
} as const satisfies Record<string, { title: string; subtitle: string; language: string }>;
export type Language = keyof typeof translations; // 'en' | 'zh'
```
Then `getAvailableLanguages` can return `Language[]` instead of `string[]` for stronger type safety.
</issue_to_address>
### Comment 2
<location path="lib/translations.ts" line_range="17-21" />
<code_context>
+
+export type Language = keyof typeof translations;
+
+export function getTranslation(lang: string) {
+ return translations[lang] ?? translations["en"];
+}
</code_context>
<issue_to_address>
**suggestion:** The `getTranslation` signature could better reflect supported languages and its return shape.
Since you already expose a `Language` type and the translations shape is fixed, you can make the function more type-safe and ergonomic by tightening the signature, e.g.:
```ts
type Translation = (typeof translations)[Language];
export function getTranslation(lang: Language | string): Translation {
return translations[lang as Language] ?? translations.en;
}
```
This keeps `Language`-typed callers fully checked while still accepting arbitrary strings (e.g. from URL params) and always returning a valid translation object.
```suggestion
export type Language = keyof typeof translations;
export type Translation = (typeof translations)[Language];
export function getTranslation(lang: Language | string): Translation {
return translations[lang as Language] ?? translations.en;
}
```
</issue_to_address>
### Comment 3
<location path="lib/translations.ts" line_range="23" />
<code_context>
+ return translations[lang] ?? translations["en"];
+}
+
+export function getAvailableLanguages(): string[] {
+ return Object.keys(translations);
+}
</code_context>
<issue_to_address>
**suggestion:** Returning `string[]` for available languages is looser than necessary and can be narrowed to the `Language` union.
If `Language` is the literal union of translation keys, this can return `Language[]`:
```ts
export function getAvailableLanguages(): Language[] {
return Object.keys(translations) as Language[];
}
```
This tightens the type so consumers can’t pass arbitrary strings where only supported languages are allowed.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| export const translations: Record< | ||
| string, |
There was a problem hiding this comment.
issue: The explicit Record<string, ...> annotation widens Language to string and loses the concrete language union.
Because translations is explicitly typed as Record<string, { ... }>, keyof typeof translations is just string, so Language is widened to string instead of the intended union of concrete language codes. This removes useful type checking and allows any string to be passed where a known language is expected.
You can keep the literal key union by relying on inference and as const satisfies, for example:
export const translations = {
en: { ... },
zh: { ... },
} as const satisfies Record<string, { title: string; subtitle: string; language: string }>;
export type Language = keyof typeof translations; // 'en' | 'zh'Then getAvailableLanguages can return Language[] instead of string[] for stronger type safety.
| export type Language = keyof typeof translations; | ||
|
|
||
| export function getTranslation(lang: string) { | ||
| return translations[lang] ?? translations["en"]; | ||
| } |
There was a problem hiding this comment.
suggestion: The getTranslation signature could better reflect supported languages and its return shape.
Since you already expose a Language type and the translations shape is fixed, you can make the function more type-safe and ergonomic by tightening the signature, e.g.:
type Translation = (typeof translations)[Language];
export function getTranslation(lang: Language | string): Translation {
return translations[lang as Language] ?? translations.en;
}This keeps Language-typed callers fully checked while still accepting arbitrary strings (e.g. from URL params) and always returning a valid translation object.
| export type Language = keyof typeof translations; | |
| export function getTranslation(lang: string) { | |
| return translations[lang] ?? translations["en"]; | |
| } | |
| export type Language = keyof typeof translations; | |
| export type Translation = (typeof translations)[Language]; | |
| export function getTranslation(lang: Language | string): Translation { | |
| return translations[lang as Language] ?? translations.en; | |
| } |
| return translations[lang] ?? translations["en"]; | ||
| } | ||
|
|
||
| export function getAvailableLanguages(): string[] { |
There was a problem hiding this comment.
suggestion: Returning string[] for available languages is looser than necessary and can be narrowed to the Language union.
If Language is the literal union of translation keys, this can return Language[]:
export function getAvailableLanguages(): Language[] {
return Object.keys(translations) as Language[];
}This tightens the type so consumers can’t pass arbitrary strings where only supported languages are allowed.
|
View changes in DiffLens |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| export const translations: Record< | ||
| string, | ||
| { title: string; subtitle: string; language: string } | ||
| > = { | ||
| en: { | ||
| title: "Coming Soon", | ||
| subtitle: "Something amazing is on the way", | ||
| language: "English", | ||
| }, | ||
| zh: { | ||
| title: "即将上线", | ||
| subtitle: "精彩内容即将呈现", | ||
| language: "中文", | ||
| }, | ||
| }; | ||
|
|
||
| export type Language = keyof typeof translations; |
There was a problem hiding this comment.
🟡 Exported Language type resolves to string due to explicit Record<string, ...> annotation
The translations object is annotated as Record<string, { ... }> at lib/translations.ts:1-3, which causes export type Language = keyof typeof translations (line 17) to resolve to string instead of "en" | "zh". In the original code in app/page.tsx, the translations object had no explicit type annotation, so Language was correctly inferred as "en" | "zh". Now, useState<Language>("en") at app/page.tsx:7 is effectively useState<string>, and the cast as Language at app/page.tsx:28 is a no-op. This silently removes compile-time safety against passing invalid language keys.
| export const translations: Record< | |
| string, | |
| { title: string; subtitle: string; language: string } | |
| > = { | |
| en: { | |
| title: "Coming Soon", | |
| subtitle: "Something amazing is on the way", | |
| language: "English", | |
| }, | |
| zh: { | |
| title: "即将上线", | |
| subtitle: "精彩内容即将呈现", | |
| language: "中文", | |
| }, | |
| }; | |
| export type Language = keyof typeof translations; | |
| const translations = { | |
| en: { | |
| title: "Coming Soon", | |
| subtitle: "Something amazing is on the way", | |
| language: "English", | |
| }, | |
| zh: { | |
| title: "即将上线", | |
| subtitle: "精彩内容即将呈现", | |
| language: "中文", | |
| }, | |
| } as const satisfies Record< | |
| string, | |
| { title: string; subtitle: string; language: string } | |
| >; | |
| export type Language = keyof typeof translations; |
Was this helpful? React with 👍 or 👎 to provide feedback.
| export function getTranslation(lang: string) { | ||
| return translations[lang] ?? translations["en"]; | ||
| } |
There was a problem hiding this comment.
📝 Info: getTranslation fallback is safe at runtime despite type widening
Despite the Language type being widened to string (reported as a bug), the getTranslation function at lib/translations.ts:19-21 uses ?? to fall back to translations["en"] for any unrecognized key. Since the select element in app/page.tsx:31-32 only offers "en" and "zh" as values, and the fallback handles any hypothetical invalid value, there is no runtime crash risk from this type regression — only a loss of compile-time safety.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export function searchGames(games: Game[], query: string): Game[] { | ||
| const lowerQuery = query.toLowerCase(); | ||
| return games.filter( | ||
| (game) => | ||
| game.title.toLowerCase().includes(lowerQuery) || | ||
| game.description.toLowerCase().includes(lowerQuery) | ||
| ); | ||
| } |
There was a problem hiding this comment.
🚩 searchGames returns all games when query is empty string
searchGames at lib/games.ts:17-24 calls query.toLowerCase() and checks includes(lowerQuery). When query is "", "".toLowerCase() is "", and String.prototype.includes("") always returns true, so every game matches. This may or may not be the intended behavior — it's fine if callers treat an empty query as "show all", but could be surprising if callers expect an empty query to return no results. Worth documenting the contract.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export function paginateGames( | ||
| games: Game[], | ||
| page: number, | ||
| pageSize: number | ||
| ): Game[] { | ||
| const start = (page - 1) * pageSize; | ||
| return games.slice(start, start + pageSize); | ||
| } |
There was a problem hiding this comment.
📝 Info: paginateGames uses 1-based page indexing without validation
paginateGames at lib/games.ts:30-37 computes start = (page - 1) * pageSize, which assumes 1-based page numbers. If page is 0 or negative, start becomes negative, but Array.prototype.slice with a negative start counts from the end of the array, returning unexpected results. This isn't necessarily a bug (callers are expected to pass valid page numbers), but the contract is implicit and could bite future callers.
Was this helpful? React with 👍 or 👎 to provide feedback.
| export const translations: Record< | ||
| string, | ||
| { title: string; subtitle: string; language: string } | ||
| > = { | ||
| en: { | ||
| title: "Coming Soon", | ||
| subtitle: "Something amazing is on the way", | ||
| language: "English", | ||
| }, | ||
| zh: { | ||
| title: "即将上线", | ||
| subtitle: "精彩内容即将呈现", | ||
| language: "中文", | ||
| }, | ||
| }; |
There was a problem hiding this comment.
📝 Info: translations object is mutable and could be modified at runtime
The translations object at lib/translations.ts:1-15 is exported as a mutable Record. Any consumer can do translations["en"].title = "Hacked" and affect all subsequent calls to getTranslation. Using as const satisfies Record<...> (as suggested in the bug fix) or making the export readonly would prevent accidental mutation. Low risk in a small codebase but worth noting for defensive design.
Was this helpful? React with 👍 or 👎 to provide feedback.
…isort, Prettier, RuboCop, Ruff Formatter, Rustfmt, Scalafmt, StandardJS, StandardRB, swift-format and Yapf This commit fixes the style issues introduced in 1fe91c3 according to the output from Autopep8, Black, ClangFormat, dotnet-format, isort, Prettier, RuboCop, Ruff Formatter, Rustfmt, Scalafmt, StandardJS, StandardRB, swift-format and Yapf. Details: #51
|
Seems you are using me but didn't get OPENAI_API_KEY seted in Variables/Secrets for this repo. you could follow readme for more information |
|
Skipping PR review because a bot author is detected. If you want to trigger CodeAnt AI, comment |
|
View changes in DiffLens |
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Code Review by LlamaPReview
🎯 TL;DR & Recommendation
Recommendation: Approve with suggestions
This PR successfully integrates CodSpeed benchmarks and extracts reusable logic into lib/ modules. Two minor maintainability improvements are suggested to reduce type duplication and strengthen type safety.
📄 Documentation Diagram
This diagram documents the refactored translation lookup flow, with the logic extracted from the page component into a dedicated module.
sequenceDiagram
participant Page as app/page.tsx
participant Trans as lib/translations
Page->>Trans: getTranslation(lang)
note over Trans: Extracted from inline code<br/>in PR #35;51
Trans-->>Page: translations[lang] or default
🌟 Strengths
- Well-structured extraction of business logic into testable modules, improving code organization and enabling performance benchmarking without UI dependencies.
💡 Suggestions (P2)
lib/games.ts: TheGameinterface duplicates an identical type incomponents/GameCard.tsx, risking drift over time. ImportingGamefromlib/games.tswould centralize the type definition.lib/translations.ts: ThegetTranslationparameter accepts a genericstringinstead of theLanguagetype, weakening compile-time safety. Changing toLanguagewould catch mismatches earlier.
💡 Have feedback? We'd love to hear it in our GitHub Discussions.
✨ This review was generated by LlamaPReview Advanced, which is free for all open-source projects. Learn more.
| export interface Game { | ||
| id: number; | ||
| title: string; | ||
| description: string; | ||
| image: string; | ||
| category: string; | ||
| } |
There was a problem hiding this comment.
P2 | Confidence: High
The Game interface in lib/games.ts is structurally identical to the inline type defined in GameCardProps within components/GameCard.tsx (see related_context). This duplication introduces a maintenance burden: any future addition or modification to the game shape (e.g., adding a releaseDate field) must be replicated in both places, increasing the risk of inconsistencies. The presence of the explicit type definition in lib/games.ts makes it the natural single source of truth. Refactoring GameCard.tsx to import Game from lib/games.ts would eliminate duplication and improve long-term maintainability.
Code Suggestion:
// components/GameCard.tsx
import type { Game } from "@/lib/games";
interface GameCardProps {
game: Game;
}Evidence: path:components/GameCard.tsx
| export function getTranslation(lang: string) { | ||
| return translations[lang] ?? translations["en"]; | ||
| } |
There was a problem hiding this comment.
P2 | Confidence: High
The getTranslation function accepts lang: string, which discards the compile-time safety provided by the Language type (keyof typeof translations). While the only current caller (app/page.tsx) narrows its argument using the Language type, custom type assertion e.target.value as Language cannot guarantee correctness if a new language code is added to translations but the <option> elements are not updated. Accepting Language directly in the function signature would catch such mismatches at compile time and make the API more self-documenting. This is a minor degradation in type safety compared to the previous inline code, where translations[lang] was only indexed with a Language-typed variable.
| export function getTranslation(lang: string) { | |
| return translations[lang] ?? translations["en"]; | |
| } | |
| export function getTranslation(lang: Language) { | |
| return translations[lang] ?? translations["en"]; | |
| } |
There was a problem hiding this comment.
Gates Passed
See analysis details in CodeScene
Quality Gate Profile: Pay Down Tech Debt
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
|
View changes in DiffLens |
 |
|
Overall Grade |
Security Reliability Complexity Hygiene |
Code Review Summary
| Analyzer | Status | Updated (UTC) | Details |
|---|---|---|---|
| JavaScript |  |
May 25, 2026 2:05a.m. | Review ↗ |
| Python |  |
May 25, 2026 2:05a.m. | Review ↗ |
| Rust | |
May 25, 2026 2:05a.m. | Review ↗ |
| Secrets | |
May 25, 2026 2:05a.m. | Review ↗ |
| Ruby | |
May 25, 2026 2:05a.m. | Review ↗ |
| Shell | |
May 25, 2026 2:05a.m. | Review ↗ |
| Scala | |
May 25, 2026 2:05a.m. | Review ↗ |
| SQL | |
May 25, 2026 2:05a.m. | Review ↗ |
| Terraform | |
May 25, 2026 2:05a.m. | Review ↗ |
| Code coverage |  |
May 25, 2026 2:05a.m. | Review ↗ |
| Swift | |
May 25, 2026 2:05a.m. | Review ↗ |
| C & C++ | |
May 25, 2026 2:05a.m. | Review ↗ |
| C# | |
May 25, 2026 2:05a.m. | Review ↗ |
| Ansible | |
May 25, 2026 2:05a.m. | Review ↗ |
Important
AI Review is run only on demand for your team. We're only showing results of static analysis review right now. To trigger AI Review, comment @deepsourcebot review on this thread.
![corgea[bot]](https://avatars.githubusercontent.com/in/673913?s=60&v=4){kind=small}
| page: number, | ||
| pageSize: number, | ||
| ): Game[] { | ||
| const start = (page - 1) * pageSize; |
There was a problem hiding this comment.
🧹 Quality - The "paginateGames" function fails to validate "page" and "pageSize", causing incorrect array slicing, such as negative start indexes when "page=0". This misaligns the control flow from its intended pagination logic. View in Corgea ↗
More Details
🎟️Issue Explanation: The "paginateGames" function fails to validate "page" and "pageSize", causing incorrect array slicing, such as negative start indexes when "page=0". This misaligns the control flow from its intended pagination logic.
- Negative or zero "page" causes "start" to be negative, making "slice" return unexpected results by counting from the array's end.
- Passing non-integer or negative "pageSize" leads to inconsistent slicing behavior, breaking assumptions in pagination.
- This flaw increases debugging complexity and reduces reliability, as pagination outputs don't align with expected page boundaries.
🪄Fix Explanation: Added input validation and normalization for pagination parameters to prevent unexpected behavior and ensure the function only processes valid positive integers.
Input validation with "Number.isFinite" ensures that non-numeric or infinite values immediately return an empty array, preventing runtime errors.
Flooring page and pageSize via "Math.floor" normalizes fractional inputs, improving consistency and expected pagination behavior.
Checks for positive values ("safePage < 1 || safePageSize < 1") guard against invalid pagination requests like zero or negative numbers.
Calculating "start" with validated and normalized inputs guarantees the slice range is always valid, improving robustness.
💡Important Instructions: Audit other pagination or indexing functions to apply similar input validation patterns for consistent robustness across the codebase.
| const start = (page - 1) * pageSize; | |
| // Validate inputs: require finite positive integers; return empty array if invalid | |
| if (!Number.isFinite(page) || !Number.isFinite(pageSize)) { | |
| return []; | |
| } | |
| const safePage = Math.floor(page); | |
| const safePageSize = Math.floor(pageSize); | |
| if (safePage < 1 || safePageSize < 1) { | |
| return []; | |
| } | |
| const start = (safePage - 1) * safePageSize; | |
| return games.slice(start, start + safePageSize); |
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (8 files)
Reviewed by laguna-m.1-20260312:free · 485,768 tokens |
1 participant
This PR integrates CodSpeed for continuous performance testing of the Games project.
What changed
Benchmark harness: Added vitest with the
@codspeed/vitest-pluginto measure performance using CPU simulation mode. This provides deterministic, low-variance measurements that are reproducible across CI runs.Extracted utility modules: Moved translation logic and game catalog operations into dedicated
lib/modules (lib/translations.tsandlib/games.ts). This makes the business logic testable and benchmarkable without requiring a browser or React rendering environment. The page component (app/page.tsx) now imports from these modules.13 benchmarks covering:
CI workflow: A new
.github/workflows/codspeed.ymlruns benchmarks on every push tomainand on pull requests, usingCodSpeedHQ/action@v4with OIDC authentication.README badge: Added a CodSpeed badge alongside the existing project badges.
Files added/modified
lib/translations.tslib/games.tsbench/games.bench.tsvitest.config.mts.github/workflows/codspeed.ymlapp/page.tsxlib/translations.tspackage.json/package-lock.jsonvitestand@codspeed/vitest-plugindev dependenciesREADME.mdNext steps
mainand report regressions on future pull requests..bench.tsfile in thebench/directory will be picked up automatically.Note on runners
This repository belongs to a personal GitHub account, so the workflow uses standard
ubuntu-latestrunners. Performance variance will be higher than with CodSpeed Macro Runners, which are only available for organization repositories. Since this setup uses CPU simulation mode (not walltime), measurement stability is still good -- simulation mode is largely insensitive to host hardware variability.Summary by Sourcery
Integrate CodSpeed-based performance benchmarking and refactor shared logic into reusable modules consumable by both the UI and benchmarks.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests:
Summary by cubic
Adds CodSpeed performance benchmarks and CI to catch regressions. Extracts translations and game catalog logic into
lib/to benchmark without the UI, and applies repo-wide formatting.New Features
vitest+@codspeed/vitest-pluginin simulation mode.lib/translations.tsandlib/games.ts;app/page.tsxnow imports fromlib/translations..github/workflows/codspeed.ymlruns on push tomainand PRs viaCodSpeedHQ/action@v4(Node 22).Dependencies
vitest,@codspeed/vitest-plugin.Written for commit 2eb3db3. Summary will update on new commits. Review in cubic