fix: use deterministic hash-based jitter for country centroid fallback#236
fix: use deterministic hash-based jitter for country centroid fallback#236haosenwang1018 wants to merge 2 commits into
Conversation
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]>
|
@haosenwang1018 is attempting to deploy a commit to the Elie Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you @haosenwang1018 can you hanle the below pls : PR #236 Review: fix: use deterministic hash-based jitter for country centroid fallbackSummaryTwo commits: (1) reduce unrest dedup rounding from 0.5° to 0.1°, (2) replace Issues Found1. Bug — JSDoc documents wrong output range The return ((hash & 0x7fffffff) / 0x7fffffff - 0.5) * 2;
// [0, 1.0] [-0.5, 0.5] [-1.0, 1.0]The 2. Redundant commit — unrest dedup change already in Commit 1 ( What's Good
VerdictRequest changes. The deterministic jitter approach (commit 2) is the right solution. Two action items before merge:
|
|
CI failures. Closing per policy. |
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.
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