fix(reorder): allow fractional and negative positions for task reorde…

[2witstudios]

…ring

The Zod validation schema required newPosition to be a non-negative integer, but the frontend uses fractional positioning (averaging neighbor positions) when reordering tasks between items. This caused validation failures when reordering task lists with more than 2 tasks.

• Remove .int() and .gte(0) constraints from reorderSchema
• Add .finite() to prevent Infinity/NaN values
• Update tests to verify fractional (2.5) and negative (-1) positions work

Summary by CodeRabbit

• Bug Fixes
  • Improved page reordering validation to accept negative and fractional position values, while maintaining rejection of invalid non-finite values.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, you can upgrade your account or add credits to your account and enable them for code reviews in your settings.

[coderabbitai]

📝 Walkthrough

Walkthrough

The pull request modifies the PATCH /api/pages/reorder endpoint to relax newPosition validation. Previously requiring non-negative integers, the endpoint now accepts any finite numeric value, including negative and fractional numbers. Corresponding tests are updated to verify this new validation behavior and confirm non-finite values are still rejected.

Changes

Cohort / File(s)	Summary
Page reorder endpoint validation apps/web/src/app/api/pages/reorder/route.ts	Changed newPosition validation from z.number().int().gte(0) to z.number().finite(), removing integer and non-negative constraints to allow any finite numeric value
Page reorder endpoint tests apps/web/src/app/api/pages/reorder/__tests__/route.test.ts	Replaced prior 400-rejection test for negative positions with new tests expecting 200 for negative (-1) and fractional (2.5) values; added test for non-finite (Infinity) values expecting 400; verified service invocation with exact newPosition values

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

• PageSpace#76: Adds comprehensive test coverage for the PATCH /api/pages/reorder route with earlier non-negative integer validation expectations, directly intersecting with this PR's validation and test changes.

Poem

🐰 Negative numbers hopping free,
Fractions dancing wild with glee!
Finite bounds keep chaos at bay,
Pages reorder every which way. ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 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 (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: allowing fractional and negative positions in task reordering, which aligns with the core validation schema modification from integer/non-negative constraints to finite numbers.

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

✨ Finishing touches

• 📝 Generate docstrings

---

🧹 Recent nitpick comments

apps/web/src/app/api/pages/reorder/__tests__/route.test.ts (1)

190-199: Consider adding test cases for NaN and -Infinity.

The test covers Infinity, but for completeness, consider adding test cases for other non-finite values that .finite() should reject.

🧪 Suggested additional test cases

it('returns 400 for NaN position values', async () => {
  const response = await PATCH(createRequest({
    pageId: mockPageId,
    newParentId: null,
    newPosition: NaN,
  }));

  expect(response.status).toBe(400);
  expect(pageReorderService.reorderPage).not.toHaveBeenCalled();
});

it('returns 400 for negative infinity position values', async () => {
  const response = await PATCH(createRequest({
    pageId: mockPageId,
    newParentId: null,
    newPosition: -Infinity,
  }));

  expect(response.status).toBe(400);
  expect(pageReorderService.reorderPage).not.toHaveBeenCalled();
});

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ce6b7e4 and 32c817d.

📒 Files selected for processing (2)

• apps/web/src/app/api/pages/reorder/__tests__/route.test.ts
• apps/web/src/app/api/pages/reorder/route.ts

🧰 Additional context used

📓 Path-based instructions (5)

apps/web/src/app/**/route.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 route handlers, params in dynamic routes are Promise objects and MUST be awaited before destructuring
Use Response.json() or NextResponse.json() for returning JSON from route handlers
Get request body using const body = await request.json();
Get search parameters using const { searchParams } = new URL(request.url);

apps/web/src/app/**/route.{ts,tsx}: In Next.js 15 dynamic routes, params are Promise objects and MUST be awaited before destructuring: const { id } = await context.params;
In Route Handlers, get request body with const body = await request.json();
In Route Handlers, get search parameters with const { searchParams } = new URL(request.url);
In Route Handlers, return JSON using Response.json(data) or NextResponse.json(data)
For permission logic, use centralized functions from @pagespace/lib/permissions: getUserAccessLevel(), canUserEditPage()

Files:

• apps/web/src/app/api/pages/reorder/route.ts

**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{ts,tsx}: Never use any types - always use proper TypeScript types
Use camelCase for variable and function names
Use UPPER_SNAKE_CASE for constants
Use PascalCase for type and enum names
Use kebab-case for filenames, except React hooks (camelCase with use prefix), Zustand stores (camelCase with use prefix), and React components (PascalCase)
Lint with Next/ESLint as configured in apps/web/eslint.config.mjs
Message content should always use the message parts structure with { parts: [{ type: 'text', text: '...' }] }
Use centralized permission functions from @pagespace/lib/permissions (e.g., getUserAccessLevel, canUserEditPage) instead of implementing permission logic locally
Always use Drizzle client from @pagespace/db package for database access
Use ESM modules throughout the codebase

**/*.{ts,tsx}: Never use any types - always use proper TypeScript types
Write code that is explicit over implicit and self-documenting

Files:

• apps/web/src/app/api/pages/reorder/route.ts
• apps/web/src/app/api/pages/reorder/__tests__/route.test.ts

**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

**/*.ts: React hook files should use camelCase matching the exported hook name (e.g., useAuth.ts)
Zustand store files should use camelCase with use prefix (e.g., useAuthStore.ts)

Files:

• apps/web/src/app/api/pages/reorder/route.ts
• apps/web/src/app/api/pages/reorder/__tests__/route.test.ts

**/*.{ts,tsx,js,jsx,json}

📄 CodeRabbit inference engine (AGENTS.md)

Format code with Prettier

Files:

• apps/web/src/app/api/pages/reorder/route.ts
• apps/web/src/app/api/pages/reorder/__tests__/route.test.ts

apps/web/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

apps/web/src/**/*.{ts,tsx}: Use message parts structure for message content: { parts: [{ type: 'text', text: '...' }] }
For database access, always use Drizzle client from @pagespace/db: import { db, pages } from '@pagespace/db';
Use centralized Drizzle ORM with PostgreSQL for all database operations - no direct SQL or other ORMs
Use Socket.IO for real-time collaboration features - imported from the realtime service at port 3001
Use Vercel AI SDK with async/await for all AI operations and streaming
Use Next.js 15 App Router and TypeScript for all routes and components

Files:

• apps/web/src/app/api/pages/reorder/route.ts
• apps/web/src/app/api/pages/reorder/__tests__/route.test.ts

🧬 Code graph analysis (1)

apps/web/src/app/api/pages/reorder/__tests__/route.test.ts (2)

apps/web/src/app/api/pages/reorder/route.ts (1)

• PATCH (16-69)

apps/web/src/services/api/page-reorder-service.ts (1)

• pageReorderService (34-153)

🔇 Additional comments (3)

apps/web/src/app/api/pages/reorder/__tests__/route.test.ts (2)

164-175: Test for negative positions looks good.

The test correctly verifies that negative positions are now accepted and that the service receives the exact value passed.

---

177-188: Test for fractional positions correctly validates the fix.

This test directly addresses the PR objective - validating that fractional positions like 2.5 (from averaging neighbor positions) work correctly.

apps/web/src/app/api/pages/reorder/route.ts (1)

10-14: Schema change appropriately supports fractional positioning.

The relaxed validation from .int().gte(0) to .finite() correctly supports the frontend's fractional positioning strategy. The .finite() constraint rejects Infinity, -Infinity, and NaN while allowing valid fractional and negative numbers.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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