improvement(geoloc): drop the dead ORDER BY size_coef on GET /geoloc/entrances

[ClemRz]

Removes the ORDER BY size_coef DESC from both entrance-in-bounds queries on GET /api/v1/geoloc/entrances. Two deleted lines plus a comment explaining why they must not come back.

Closes #1821.

🤔 What

• The clause is gone from both query variants. PUBLIC_ENTRANCES_IN_BOUNDS and PUBLIC_ENTRANCES_IN_BOUNDS_AND_MASSIF (api/services/GeoLocService.js). The ?massif= variant matters as much as the plain one — it backs the massif page, the consumer most likely to sit over dense karst.
• The response is otherwise byte-identical. Same rows, same columns, same quality field. Only the array order becomes unspecified.
• A comment records why. Without it the clause reads like an oversight and gets restored, which silently restores the sort node.

🤷‍♂️ Why

• It forced the sort that spilled to disk in perf(db): geoloc entrances sort spills to disk on large bounding boxes (work_mem undersized) #1812. The sort sat above every join, so it carried all 25 projected columns — ~205 bytes per row. Past roughly 18 000 rows that exceeds the server's work_mem and Postgres falls back to an external merge on the data volume. There is no other reason for a sort node in this query, so deleting the clause makes the spill structurally impossible at any bounding box size.
• The LIMIT it existed to serve never truncates. It is hardcoded at 100000 (find-entrances.js:28), while MAX_BBOX_AREA_KM2 (bug(api): unbounded bbox on GET /api/v1/geoloc/entrances blocks the event loop and degrades every route #1810) holds any legal box to roughly 16 000 entrances. Probing /geoloc/countEntrances for the densest box that still passes at 34 900 km² — a north-south strip along the Jura/Alps axis — returns 15 834 rows. A result set that cannot reach the limit means the ordering could never change which rows were returned.
• size_coef is not a ranking. calcule_size_coef (sql/0_triggers.sql:7) sums a depth bucket (0/4/5) and a length bucket (0/1/2/4/5), so it has at most 11 values. The live response for that box confirms it: quality carries only 10,9,8,7,6,5,4,2,1,0,null across 15 834 rows.
• It ranked the wrong way round anyway. t_entrance.id_cave is nullable, so a cave-less entrance has a NULL size_coef, and NULLS FIRST is the default under DESC. The first row of that live response is {"caveId":null,"quality":null,…} — had the LIMIT ever truncated, it would have preferentially kept the cave-less entrances, the opposite of the intent.
• Nothing depends on the order. mapTileCache.computeUnion concatenates tiles in Map-iteration order, so cross-tile order was already arbitrary; MapClusters filters by quality without taking a prefix; MapMassif.jsx and useNearbyEntrances pass the array straight through; reducers/Map.js stores it verbatim. Swagger documents no ordering guarantee.
• It avoids a work_mem change on a 4 GiB box. The alternative fix was raising work_mem, but it applies per node: the perf(db): geoloc entrances sort spills to disk on large bounding boxes (work_mem undersized) #1812 plan claims 4 MB (sort) + 3 × 8 MB (hashes, hash_mem_multiplier = 2) ≈ 28 MB for one connection. At 32 MB that becomes ~224 MB per connection, with max_connections = 429, on a Standard_B2s already committing 1 GiB to shared_buffers. This PR needs no server-parameter change and carries no Production label.

🔍 How

Deleted the two ORDER BY size_coef DESC lines. grep -rn "ORDER BY size_coef" api/ now matches only the explanatory comment.

NETWORKS_IN_BOUNDS keeps its own unrelated ORDER BY c.id, en.id — different query, different endpoint, untouched. Every other geoloc query never had the clause.

🧪 Testing

• npx eslint api/services/GeoLocService.js — clean.
• GeoLocService.test.js, GeoLocNameFilter.test.js, Geoloc.test.js — 105 passing, 0 failing. These already assert the returned set for a bbox, order-independently, which is what proves there is no behavioural regression.

No new test. The only direct guard would assert the absence of a Sort node via EXPLAIN, and that is unsound: on the small test dataset the planner can legitimately introduce a sort for a merge join, so the test would fail for reasons unrelated to this clause. The comment in the source carries the invariant instead.

Manual check against production, for the record — the densest legal bbox:

GET /api/v1/geoloc/entrances?sw_lat=44.5000&sw_lng=5.3227&ne_lat=47.5000&ne_lng=6.6773
→ 200, 15 834 rows, 4 211 767 bytes

📸 Previews

None — no user-visible change. The map renders the same markers; only the order they arrive in is no longer specified.

Out of scope

That same request takes 4.4–6.1 s for a 4.2 MB response. That is row volume and Node serialisation — the #1810 mechanism, not the sort — and no option in #1812 or #1821 addresses it. It needs its own issue: lower the cap, paginate, or stop shipping 14 data-quality columns per row to a map that renders one.

[Paul-AUB]

Code Review — fix/1821-drop-dead-order-by

Review target: branch vs origin/develop (source: override)

Scope: single file, api/services/GeoLocService.js — removal of
ORDER BY size_coef DESC from PUBLIC_ENTRANCES_IN_BOUNDS and
PUBLIC_ENTRANCES_IN_BOUNDS_AND_MASSIF, plus an explanatory comment.

Note: the review was invoked as issue 1822 but the branch name and the
in-code comment both reference #1821. The diff matches issue 1821.

Findings

No P0 / P1 / P2 findings.

Verifications performed

• Comment accuracy — id_cave nullability. api/models/TEntrance.js:244-247
  declares cave with no required: true, so id_cave is nullable. The
  LEFT JOIN t_cave therefore yields NULL size_coef for cave-less entrances,
  and PostgreSQL's NULLS FIRST default under DESC is standard — the
  "wrong way round" claim holds.
• Comment accuracy — hardcoded limit. api/controllers/v1/geoloc/find-entrances.js:28
  passes 100000 to getEntrancesMap, matching the "hardcoded at 100 000"
  claim in the comment.
• No internal consumer of order. The only caller is find-entrances.js,
  which forwards rows straight to res.json — no top-N truncation, no slicing,
  no aggregation that would care about row order.
• No test relies on order. test/integration/1_services/GeoLocService.test.js
  and test/integration/1_services/GeoLocNameFilter.test.js assert shape,
  cardinality, and uniqueness — never ordering.
• No public contract violated. assets/swaggerV1.yaml:5204-5279 documents
  the /geoloc/entrances response as an unordered array; no ordering guarantee
  is advertised.

P3-adjacent observation (not raised as a finding)

A frontend that rendered rows in received order will now see a different
visual order. This is addressed directly by the added comment, which points
maintainers at the client-side sort using the quality field that
formatEntrances (line 269) still populates from size_coef on every row.
Absent concrete evidence of a frontend regression in this repo, it does not
meet the finding bar under the skill's no-speculation rule.

Final Summary

• Overall correctness: correct
• Overall explanation: The diff removes a genuinely dead sort clause and
  documents why it was harmful; every factual claim in the added comment is
  verifiable from the surrounding code, and no in-repo consumer or test
  depends on the removed ordering.
• Overall confidence: 0.9