Skip to content

Add isOdd factory SDK util#277

Closed
kjgbot wants to merge 1 commit into
mainfrom
ar-235-is-odd
Closed

Add isOdd factory SDK util#277
kjgbot wants to merge 1 commit into
mainfrom
ar-235-is-odd

Conversation

@kjgbot

@kjgbot kjgbot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a pure isOdd(n) helper in packages/factory-sdk/src/is-odd.ts
  • Add Vitest coverage for odd, even, and zero inputs

Validation

  • npx vitest run packages/factory-sdk/src/is-odd.test.ts

Linear: AR-235

@coderabbitai

coderabbitai Bot commented Jun 13, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: cf2e063c-1e62-4803-8278-ad4b6150fde7

📥 Commits

Reviewing files that changed from the base of the PR and between de24cb3 and 08c15b7.

📒 Files selected for processing (2)
  • packages/factory-sdk/src/is-odd.test.ts
  • packages/factory-sdk/src/is-odd.ts

📝 Walkthrough

Walkthrough

This PR introduces the isOdd utility function to the factory-sdk package, which determines whether a number is odd by checking the modulo result. The implementation is accompanied by a complete test suite covering odd numbers, even numbers, and zero.

Changes

isOdd utility function

Layer / File(s) Summary
isOdd function and tests
packages/factory-sdk/src/is-odd.ts, packages/factory-sdk/src/is-odd.test.ts
Exports isOdd(n: number): boolean using modulo arithmetic (n % 2 !== 0), with three test cases validating true for 1, false for 2, and false for 0.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

🐰 A function so neat, modulo-sweet,
Checking if numbers are odd on their feet!
One test, two, three—the suite looks so keen,
The simplest utility the farm's ever seen. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding an isOdd utility function to the factory SDK.
Description check ✅ Passed The description is directly related to the changeset, providing a clear summary of changes and validation instructions.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch ar-235-is-odd

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint install failed due to a network error.


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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@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 introduces a new isOdd utility function and its corresponding unit tests. Feedback on the implementation suggests ensuring the input is a valid integer using Number.isInteger to prevent incorrect results for non-integer values, NaN, and Infinity.

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 on lines +1 to +3
export function isOdd(n: number): boolean {
return n % 2 !== 0
}

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

The current implementation of isOdd returns true for non-integer values (such as 1.5), NaN, and Infinity because n % 2 for these values is non-zero (or NaN), which satisfies the !== 0 check. Mathematically, parity is only defined for integers.

To prevent incorrect behavior when non-integers or special number values are passed, we should ensure the input is a valid integer using Number.isInteger(n).

Suggested change
export function isOdd(n: number): boolean {
return n % 2 !== 0
}
export function isOdd(n: number): boolean {
return Number.isInteger(n) && n % 2 !== 0
}

@kjgbot

kjgbot commented Jun 13, 2026

Copy link
Copy Markdown
Contributor Author

Closing per factory close-never-merge policy: synthetic [factory-e2e] probe PR (issue is synthetic; probe PRs are close-never-merge by policy, never merged). Manual enforcement — the daemon's auto-close no-op'd because the PR title lacks the [factory-e2e] marker (close-probe bug, fix in flight).

@kjgbot kjgbot closed this Jun 13, 2026
agent-relay-code Bot added a commit that referenced this pull request Jun 13, 2026
@agent-relay-code

Copy link
Copy Markdown
Contributor

Reviewed and fixed PR #277’s current checkout.

Changes made:

  • Guarded isOdd with Number.isInteger so fractional, NaN, and infinite values are not treated as odd: is-odd.ts
  • Added regression coverage for fractional and non-finite inputs: is-odd.test.ts
  • Exported isOdd from the factory SDK entrypoint so the new SDK utility is reachable through the package API: index.ts

Addressed comments

  • coderabbitai[bot]: reported no actionable review comments; its ESLint tool had a network install warning, but local npm run lint passed. No code change needed for that comment.
  • coderabbitai[bot]: docstring coverage warning in its generated pre-merge notes. No change made: this repo/package does not use JSDoc on comparable exported SDK functions, and repo CI does not enforce that warning.
  • gemini-code-assist[bot] review summary: requested integer validation for isOdd; fixed in is-odd.ts, with tests in is-odd.test.ts.
  • gemini-code-assist[bot] inline thread on packages/factory-sdk/src/is-odd.ts: same integer-validation issue; fixed in is-odd.ts.
  • kjgbot: noted the PR was closed under factory close-never-merge policy. No code change; PR metadata now reports state closed, merged false, mergeable true.

Verification run:

  • npm ci
  • npx vitest run packages/factory-sdk/src/is-odd.test.ts
  • npm run verify:mcp-resources-drift
  • npm run lint passed with pre-existing warnings outside the PR files
  • npm run typecheck:web
  • npm run typecheck:node
  • npm test
  • npx vitest run
  • npm run build
  • npm run build:web
  • npx playwright install --with-deps chromium
  • npx playwright test --config playwright.fidelity.config.ts
  • npx playwright test --config playwright.redraw.config.ts

I did not print READY because PR #277 is currently closed/unmerged, so it is not in an open merge-ready state.

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