PR Summary

• Addition of a new JSON file
  A new file named fix-cosmos-pagination_2026-02-27-21-10.json was added, which addresses issues with database pagination. This was classified as a minor fix for the core package of our application.

• Modifications to query-helper.ts

  • Introduction of a new numerical value: Establishes a default page size of 100 entries.
  • Reworked pagination mechanics: Now, our database interactions have been improved to better manage data pagination, especially in terms of consistency and continuity.
  • Adjusted legacy querying system: Older methods of querying data are now more accommodating of how much data is shown on each page while retaining their former functionality.
  • Updated how cursor IDs are calculated: Cursor ID, which is used in moving from one page of data to another, is now based on an effective limit to ensure consistent navigation across different pages of data.

/integration sha=04d10e1

⌛ Integration tests are running...

Check their status here 👈

❌ Oh no! Integration tests have failed

⌛ Integration tests are running...

Check their status here 👈

❌ Oh no! Integration tests have failed

⌛ Integration tests are running...

Check their status here 👈

❌ Oh no! Integration tests have failed

⌛ Integration tests are running...

Check their status here 👈

❌ Oh no! Integration tests have failed

⌛ Integration tests are running...

Check their status here 👈

❌ Oh no! Integration tests have failed

/integration sha=589add3

⌛ Integration tests are running...

Check their status here 👈

✅ Integration tests have finished successfully!

Claude: Code Review — Fix Cosmos DB pagination for backwards compatibility

He revisado los 4 ficheros cambiados en este PR. Análisis completo:

Contexto y lógica del cambio

El PR #1587 introdujo paginación basada en continuationToken para Azure/CosmosDB, eliminando el id numérico del cursor. Esto rompió frontends existentes que esperaban un id numérico. Este PR restaura la compatibilidad añadiendo siempre un id numérico al cursor. El enfoque es correcto en concepto.

✅ Lo que está bien

• La idea de devolver siempre cursor.id (posición numérica) junto con el continuationToken optativo es el enfoque correcto para mantener retrocompatibilidad sin romper la nueva funcionalidad.
• Introducir DEFAULT_PAGE_SIZE = 100 es una buena práctica para evitar que el SDK de Cosmos devuelva tamaños de página no deterministas.
• La separación entre el path de continuationToken y el path legacy OFFSET sigue siendo clara.
• Los tests de integración (commit 589add3) se actualizaron para reflejar el nuevo comportamiento del cursor.

🔴 Problemas que bloquean el merge

1. parseInt(afterCursor.id) sin guard de NaN — puede producir cursores corruptos

const previousOffset = afterCursor?.id ? parseInt(afterCursor.id) : 0

Si un cliente envía un afterCursor.id no numérico (por ejemplo un cursor antiguo en formato string no-numérico, o un error del cliente), parseInt devuelve NaN. El resultado es:

• cursor.id en la respuesta vale "NaN" — string inválido que llega al cliente
• previousOffset + effectiveLimit = NaN + 100 = NaN — corrupción silenciosa

El código ya tiene esta guardia en otro lugar (!isNaN(parseInt(afterCursor.id))) en la rama legacy, pero no en la rama nueva. Solución:

const parsedId = afterCursor?.id ? parseInt(afterCursor.id, 10) : NaN
const previousOffset = Number.isFinite(parsedId) ? parsedId : 0

2. cursor.id avanza por effectiveLimit en lugar de por el número real de items devueltos

// path con continuationToken:
cursor = { continuationToken, id: (previousOffset + effectiveLimit).toString() }

// path sin continuationToken (no legacy):
cursor = { id: (previousOffset + effectiveLimit).toString() }

Cosmos DB no garantiza que devuelva exactamente maxItemCount items en cada página. Puede devolver menos si:

• Es la última página (parcialmente llena)
• Cosmos decide hacer un "fan-out" en particiones y devuelve menos de lo solicitado
• Se aplican filtros post-query

Si un cliente que solo usa cursor.id (ignorando continuationToken) hace la siguiente petición con afterCursor: { id: "100" } cuando solo se devolvieron 73 items, se saltará 27 registros.

La rama legacy del código antiguo (currentOffset + finalResources.length) era más correcta para ese caso. El fix regresa a effectiveLimit en ambos paths, lo que puede introducir data skipping peor que el bug original.

Recomendación: en el path de continuationToken, usar effectiveLimit es aceptable (el cliente debería usar el token), pero en el path sin continuationToken se debería usar finalResources.length:

} else if (finalResources.length > 0) {
  cursor = { id: (previousOffset + finalResources.length).toString() }
}

3. feedOptions.maxItemCount no se configura en la primera página del path de continuationToken cuando no hay limit

if (limit || afterCursor?.continuationToken) {
  feedOptions.maxItemCount = limit ?? DEFAULT_PAGE_SIZE
}

En la primera llamada a una query paginada con continuationToken (paginatedVersion: true, sin limit, sin afterCursor): feedOptions.maxItemCount no se establece. Cosmos devolverá un número arbitrario de items (típicamente 100, pero no garantizado), y el cursor.id calculado con effectiveLimit = 100 puede no coincidir con el número real devuelto.

Esto hace que el cursor.id sea un mentiroso desde la primera página. Solución simple y ya sugerida por Copilot:

// Siempre establecer maxItemCount en el path paginado
feedOptions.maxItemCount = limit ?? DEFAULT_PAGE_SIZE

🟡 Observaciones importantes

4. Rush change file declara framework-core en lugar de framework-provider-azure

{ "packageName": "@boostercloud/framework-core", ... }

El código que cambia está en packages/framework-provider-azure/src/helpers/query-helper.ts. Con Rush, el change file controla qué paquete recibe un version bump y aparece en el CHANGELOG. Si este change file va a framework-core, el fix nunca aparece en el changelog ni en la versión publicada de framework-provider-azure.

Los usuarios que actualicen framework-provider-azure para obtener el fix de paginación no verán nada en su CHANGELOG. Esto es un problema de trazabilidad importante para los consumidores del paquete.

Recomendación: Generar el change file con rush change --target-branch main seleccionando @boostercloud/framework-provider-azure.

5. Tests de integración fallan 4 veces antes de pasar — señal de flakiness

El historial muestra 4 rondas de "Integration tests failed" antes de que pasen con el commit 589add3. Esto puede indicar:

• Flakiness en los tests de integración (condición de carrera, timing)
• O que el fix de 589add3 corrigió algo real en los tests

Si es flakiness, debería abrirse un issue separado porque tests flaky en CI reducen la confianza en todos los PRs.

6. No hay tests unitarios para los nuevos paths

Las observaciones de Copilot ya señalan esto: no hay tests unitarios que cubran:

• Primera página con continuation-token (sin afterCursor)
• Página subsiguiente con { continuationToken, id }
• Comportamiento cuando limit está ausente (DEFAULT_PAGE_SIZE)
• Validación de que cursor.id es siempre un número válido

Los tests de integración son lentos y caros para iterar sobre lógica de paginación. Los tests unitarios de query-helper.ts deberían cubrir estos casos.

7. parseInt sin radix explícito — code style minor

parseInt(afterCursor.id)  // sin radix

Aunque en la práctica los IDs son siempre decimales, es buena práctica usar parseInt(afterCursor.id, 10) para ser explícito y evitar comportamiento inesperado con strings como "0x1f".

Veredicto

Request Changes. Los puntos 1 (NaN sin guardia), 2 (avance por effectiveLimit en lugar de items reales en el path sin token) y 3 (maxItemCount no siempre configurado) son bugs lógicos que pueden producir paginación incorrecta en producción. El punto 4 (change file en el paquete incorrecto) es un problema de release management que impide que el fix aparezca en el paquete correcto.

/integration sha=50d87eb

⌛ Integration tests are running...

Check their status here 👈

✅ Integration tests have finished successfully!

@alvaroloes implemented some of the suggestions in commit 50d87eb. Regarding the second one: The original code (commit 44045a7, before PR #1587) was:

cursor: { id: ((limit ? limit : 1) + (afterCursor?.id ? parseInt(afterCursor?.id) : 0)).toString() }

This always advanced by limit, not by actual result count. PR #1587 (cbac6d6) changed it to finalResources.length, which is what broke frontends. Our fix restores the original semantic. The reviewer's suggestion to use finalResources.length in the no-token path would actually reintroduce the broken behavior from #1587.