Skip to content

fix(api): stable ordering, bounded limit and documented pagination on v2 (#52) - #53

Merged
tomahock merged 2 commits into
mainfrom
fix/v2-pagination
Oct 26, 2026
Merged

tomahock merged 2 commits into
mainfrom
fix/v2-pagination

Conversation

@tomahock

@tomahock tomahock commented Oct 26, 2026 •

Copy link
Copy Markdown
Member

Closes #52

Audit

Endpoint Params Ordering (before) Metadata (before)
GET /v2/incidents/active limit (1000), page created_at — field doesn't exist none
GET /v2/incidents/active/kml limit (300), page created_at — doesn't exist n/a (KML)
GET /v2/incidents/search limit (50), page none paginator (undocumented)
GET /v2/incidents/1000ha-burned limit (500), page none paginator
GET /v2/photos/latest, GET /v2/incidents/{id}/photos, POST …/photos/all, GET /v2/moderation/photos per_page (clamped), page created_at meta

Without a real sort, Mongo returns natural order, so walking pages can repeat or skip incidents. limit was unbounded (limit=100000000), 0 silently became 15 (Eloquent default) and negatives reached Mongo.

Changes

Endpoints

  • Deterministic order: created desc (active, kml), dateTime desc (search, 1000ha) — both indexed — plus _id tie-break; photos also get the _id tie-break.
  • limit clamped to 1..1000; non-integer → endpoint default. Defaults unchanged.
  • /v2/incidents/active JSON response now includes paginator (additive). paginator.perPage and photo meta.last_page added (additive).

Docs

  • New page parameter on every paginated endpoint (previously only on /v2/photos/latest).
  • limit documented per endpoint with default/max and clamping; search was missing limit altogether.
  • Ordering described on each endpoint.
  • active/search now document the paginated response (v2-fires-paginated-list.yaml); Paginator model described; photo meta → shared PhotoPageMeta model.
  • POST /v2/incidents/{id}/photos/all was not documented at all — added.
  • Lint: no new problems except the project-wide security-defined rule on the new operation (all existing operations already fail it).

Kept as is (would break clients)

Two pagination styles coexist: incidents use limit + paginator{currentPage,totalPages,totalItems}, photos use per_page + meta{total,page,per_page}. Both are now documented; unifying them would be a breaking change for the apps.

⚠️ Behaviour changes for clients

  • Order of /v2/incidents/active, /active/kml, /search and /1000ha-burned is now newest-first (previously effectively insertion order).
  • limit above 1000 is capped at 1000 → clients that relied on a bigger single page must follow paginator.totalPages. (csv2 export on search is not paginated and is unaffected.)

Tests

tests/Feature/Controllers/IncidentController/PaginationTest.php (5 tests: page walk without overlap + paginator on active, limit clamping/defaults on active/search/1000ha, search order across pages, 1000ha order, photo last_page). All 5 fail on main; plus 1 index-plan test. Full suite against a throwaway MongoDB 7: 86 pass.

Indexes (Codex review)

Sorting search by (dateTime, _id) was not covered by the existing (isFire, dateTime) / (dateTime) indexes, so Mongo did a blocking in-memory sort over all matches (the whole archive for a search without dates). New migration 2026_10_26_180000_add_search_sort_indexes adds (isFire, dateTime, _id) and (dateTime, _id) on data; a test asserts the search plans use an index scan with no SORT stage (fails without the migration).

Not indexed on purpose: active sorts only the active incidents (hundreds at most), and 1000ha-burned costs the same as before (its icnf.burnArea.total filter isn't indexed and the total count already scanned every fire).

Deploy

  1. Deploy as usual.
  2. php artisan migrate --force — creates the two indexes on data (online build; may take a little while on the full archive). Without it the endpoints still work, just slower on large searches.

No env changes, no queue restart needed.

🤖 Generated with Claude Code

… v2 (#52)

- active and active/kml sorted by `created_at`, which incidents don't
  have, so pages could overlap or skip items; search and 1000ha-burned
  had no sort. Now `created` desc (active, kml) or `dateTime` desc
  (search, 1000ha), with `_id` as tie-break. Photo listings get the same
  tie-break.
- `limit` is clamped to 1..1000 (non-integer → endpoint default) instead
  of being unbounded / becoming 15 for 0 / reaching Mongo negative.
- /v2/incidents/active JSON now returns the `paginator` object;
  `paginator` gains `perPage`, photo `meta` gains `last_page`.
- OpenAPI: `page` on every paginated endpoint, `limit` with default/max
  per endpoint, ordering described, paginated response for active and
  search, shared Paginator/PhotoPageMeta models, and the missing
  POST /v2/incidents/{id}/photos/all.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 26, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T17:43:50.420703Z 987bf65 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector 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.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 987bf65596

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

})->when($subRegion, function ($query, $subRegion){
return $query->where('sub_regiao', (string)$subRegion);
});
})->orderBy('dateTime', 'desc')->orderBy('_id', 'desc');

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Add compound indexes for the new search ordering

For searches without a date restriction, this adds a blocking sort over the entire matching incident archive before returning even the first 50 records. The indexes defined in database/migrations/2026_05_25_120000_create_mongodb_indexes.php cover dateTime and (isFire, dateTime), but neither includes the new _id sort key; MongoDB therefore cannot obtain this complete ordering from those indexes (MongoDB documentation). Unlike the previous unsorted, limited query, each page now requires fetching and sorting all matches, creating a substantial latency and database-load regression on large archives. Add matching compound indexes for the default fire-only search and the all=1 search alongside this ordering change.

Useful? React with 👍 / 👎.

/v2/incidents/search now sorts by (dateTime desc, _id desc). The existing
(isFire, dateTime) and (dateTime) indexes don't include _id, so Mongo
fell back to a blocking in-memory sort over every match, e.g. the whole
fire archive for a search without dates. Add (isFire, dateTime, _id)
for the default fire-only search and (dateTime, _id) for all=1; a test
asserts the winning plan has no SORT stage.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@tomahock
tomahock merged commit b2eeb05 into main Oct 26, 2026
1 check passed
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.

v2 pagination: unstable ordering, unbounded limit, undocumented page/meta

1 participant