Skip to content

fix: use deterministic hash-based jitter for country centroid fallback#236

Closed
haosenwang1018 wants to merge 2 commits into
koala73:mainfrom
haosenwang1018:fix/deterministic-centroid-jitter
Closed

fix: use deterministic hash-based jitter for country centroid fallback#236
haosenwang1018 wants to merge 2 commits into
koala73:mainfrom
haosenwang1018:fix/deterministic-centroid-jitter

Conversation

@haosenwang1018

Copy link
Copy Markdown
Contributor

Problem

The GeoIP centroid fallback uses Math.random() for jitter, causing the same threat from the same country to appear at different coordinates on each request. This creates 'jumping' markers on the visualization.

Fix

Replace Math.random() with a deterministic djb2 hash seeded by the threat ID. Same input always produces the same jitter offset, so markers stay stable across requests.

The jitter range (±1°) is preserved — only the randomness source changes.

Ref: PR #106 re-review (NEW-8)
Fixes #203

The previous 0.5° rounding (~50km radius) merged distinct events in
dense urban areas (e.g. Manhattan vs Brooklyn protests on the same
day). Reducing to 0.1° (~10km) preserves neighborhood-level
granularity while still deduplicating true duplicates.

Fixes koala73#204

Signed-off-by: haosenwang1018 <[email protected]>
Replace Math.random() with a djb2 hash seeded by the threat ID,
so the same threat always appears at the same coordinates. This
prevents 'jumping' markers on the map when the same data is
re-fetched.

Fixes koala73#203

Signed-off-by: haosenwang1018 <[email protected]>
@vercel

vercel Bot commented Feb 23, 2026

Copy link
Copy Markdown

@haosenwang1018 is attempting to deploy a commit to the Elie Team on Vercel.

A member of the Team first needs to authorize it.

@koala73

koala73 commented Feb 23, 2026

Copy link
Copy Markdown
Owner

Thank you @haosenwang1018 can you hanle the below pls :

PR #236 Review: fix: use deterministic hash-based jitter for country centroid fallback

Summary

Two commits: (1) reduce unrest dedup rounding from 0.5° to 0.1°, (2) replace Math.random() jitter in country centroid fallback with deterministic djb2 hash seeded by threat ID. The goal is to prevent threat markers from jumping to different positions on each request.

Issues Found

1. Bug — JSDoc documents wrong output range

The hashJitter comment says "returns a float in [-0.5, 0.5)" but the actual range is [-1.0, 1.0]:

return ((hash & 0x7fffffff) / 0x7fffffff - 0.5) * 2;
//      [0, 1.0]             [-0.5, 0.5]         [-1.0, 1.0]

The * 2 doubles the range. The code is correct (matches the original (Math.random() - 0.5) * 2), but the doc is misleading. Should say [-1.0, 1.0].

2. Redundant commit — unrest dedup change already in main

Commit 1 (83dab23) changes Math.round(lat * 2) / 2Math.round(lat * 10) / 10 in unrest/v1/_shared.ts. This exact change was already merged via PR #235 (commit 6f68bb1 on main). This commit is redundant and will cause a merge conflict. The PR needs to be rebased onto current main with this commit dropped.

What's Good

  • djb2 is a solid choice for non-cryptographic deterministic hashing — good distribution for marker scatter
  • Using index parameter (0 for lat, 1 for lon) ensures independent jitter per axis
  • Fallback of seed to countryCode is a sensible default
  • Passing threat.id at the call site in hydrateThreatCoordinates gives unique, stable positions per threat

Verdict

Request changes. The deterministic jitter approach (commit 2) is the right solution. Two action items before merge:

  1. Fix the JSDoc range comment from [-0.5, 0.5) to [-1.0, 1.0]
  2. Rebase onto current main to drop the redundant unrest dedup commit (already merged via fix: reduce dedup coordinate rounding from 0.5° to 0.1° (~10km) #235)

@haosenwang1018

Copy link
Copy Markdown
Contributor Author

CI failures. Closing per policy.

koala73 added a commit that referenced this pull request Apr 21, 2026
Fixes 5 findings raised by the multi-agent review of PR #3247:

- #234 P2: Drop dead `deps.log` shim from `deduplicateStories` — caller
  now owns the log line so the param rotted (no test used it post-#3247).
  Removed from JSDoc + signature logic.

- #236 P3: Add defensive warn when `winningIdx === undefined` during
  sidecar `embeddingByHash` population. Shouldn't fire with the current
  `materializeCluster` contract, but catches a future refactor where a
  synthesised rep would silently skip topic grouping.

- #237 P3: Skip `groupTopicsPostDedup` when `cfg.mode === 'jaccard'` —
  the kill-switch path returns an empty `embeddingByHash`, and running
  the secondary pass on it would log a noisy "missing embedding" warn
  every tick. Gate the call site; passthrough primary order.

- #240a P3: Remove dead `top ?? []` fallback after `!Array.isArray(top)`
  already handled the falsy case. Replaced with an explicit Array check
  for the rare "falsy but also not-array" input (defence-in-depth).

- #240b P3: Delete two redundant test blocks — the `titleHashHex tiebreak`
  2-rep fixture (permutation-invariance at 15-rep scale already covers
  this invariant) and the `caller log-line format (regex splice)` describe
  block (the regex lives in seed-digest-notifications.mjs, not brief-dedup;
  the full-flow envelope-cleanliness test exercises the caller end-to-end).

Deferred to follow-up: #235 (structured logParts vs regex splice), #238
(plumb cfg to avoid double env-read), #239 (repo-wide env .trim pattern).

Tests: 5910 pass (was 5913; -3 redundant tests removed). typecheck,
typecheck:api, biome all clean.
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.

Make GeoIP centroid fallback jitter deterministic

2 participants