Feat/step0 client geolocation - #23
Merged
Merged
Conversation
Items 1 and 2: split Phase A so source-IP geolocation runs between merge and detection, and switch 02/03 off MaxMind's client.Geo onto IPInfo. Phase A split ------------- A1 step 01 (merge) -> merged_download_upload A0 run_enrichment(source="clients") -> unified_src_ip_to_geoloc A2 steps 02-03 (detection) The order is forced: client IPs come from merged_download_upload, so A0 cannot run before 01, and 02/03 group on client geography so it must run before them. A0 failure is logged, not fatal -- detection degrades rather than blocking the batch. Results are recombined so per-date reporting still lines up with `dates`. Clients skip rDNS/HOIHO: those resolve router hostnames and have nothing to do on a client IP. Client geography: IPInfo, not MaxMind ------------------------------------- MaxMind emits a single designated point per country when it cannot place a client below country level. On 2026-08-07 that was 547,105 measurements on 864 coordinates across 185 countries -- sometimes the geographic centre (India 21.997,79.001), often the capital (Tokyo, London, Paris, Jakarta, Madrid). Metro grouping piled them into whichever metro contains the point: Jabalpur-Madhya Pradesh-IN 153,690 measurements, 97.8% synthetic, 1,451 ASNs Paris-Paris-FR 33,521, 55.9% Tokyo-Tokyo-JP 73,540, 53.3% IPInfo resolves those clients into 8,066 distinct cities at a median /24.5. 02/03 now rewrite the client.Geo struct in place from unified_src_ip_to_geoloc, so all 42 downstream references read IPInfo and none can still be reading MaxMind. Replaced, not blended: a COALESCE fallback would silently reintroduce the country-centroid problem. Where IPInfo has nothing the geography is NULL, which is how MaxMind's unplaceable clients already behaved. Coverage is 99.995% of client IPs / 99.05% of measurements against the 2-month-stale BigQuery snapshot, so a lower bound on the daily MMDB. _source_metro_lookup is deleted from both files. Phase A0 already resolved every client IP to a metro, so detection does no spatial join at all -- the ST_COVERS/ST_DWITHIN work leaves 02 and 03 entirely. Behaviour change to note: Subdivision1ISOCode now carries IPInfo's full region NAME, so city-mode labels become `Paris-Ile-de-France-FR` rather than `Paris-IDF-FR`, matching the metro label shape. test_02_resolves_metro_before_statistical_grouping asserted the mechanism (_source_metro_lookup + ST_COVERS). Rewritten to assert the outcome -- the metro is settled before any statistic -- plus a new test that no MaxMind value can survive into a group label. All four CI gates pass on 3.14: ruff check, ruff format --check, mypy, pytest. Both queries plan against BigQuery in both granularity modes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ailure is fatal _download_ipinfo_database names the destination for today unconditionally but only downloads when IPInfo's checksum differs from the local one. When IPInfo has not republished since the last download, nothing is written and the returned path names a file that never existed; open_database fails, reader is None, and every lookup silently returns empty. Production never hit this because the nightly runs at 15:00 UTC, after the 08:18 refresh -- the 2026-08-12 04:26 UTC staging run did. Fall back to the newest existing snapshot on the normal path, not only in the except branch, since the failure mode is a no-op download rather than an error. Order snapshots by the date in the filename instead of getctime, which reorders under restore/docker cp/touch. Client geolocation now raises when the reader is unavailable, and Phase A0 is no longer wrapped in try/except. Both were safe to degrade while 02/03 read client.Geo.*; now that they read only unified_src_ip_to_geoloc, a failure yields a batch grouped entirely under NULL, which reads as 'no anomalies' rather than as a failure. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects surfaced by the first Phase A0 staging run. The client-candidate query hardcoded hermes_union.merged_download_upload, so a --target staging run collected client IPs from production. Staging holds a copy of the same partitions, so the leak produced correct-looking output; it is now a parameter, threaded from run_dates through run_enrichment. The transient_events override follows the same dataset, which makes Phase B honour --target too. run_enrichment ran rDNS and HOIHO regardless of source. Both infer geolocation from router hostnames, which residential CPE do not carry, both are the expensive part of enrichment (HOIHO loads a 3.7M-entry rDNS cache), and both write to the topology tables -- so Phase A0 duplicated Phase B's work and mutated topology state from a client-geolocation step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…seline window Both found by the first Phase A0 staging run, which showed transient events down 70.5% against production on identical input. Neither is a schema or row-count error -- both produce well-formed output over a silently truncated population. Enrichment writes IPv4 and IPv6 client geolocation to separate tables, since the enricher is constructed once per family with its own tables dict. Steps 02/03 read only the IPv4 table, so every IPv6 client got a NULL group label and left detection: 1,169,928 of 2,564,893 distinct client IPs on 2026-08-07 (45.6%), carrying 1,786,805 of 4,855,404 measurements (36.8%). Both steps now UNION ALL the two tables, which is sound because an address has exactly one family. Phase A0's lookback was the batch span, but step 02 compares the target day against a 7-day baseline and groups those days by client geography too. Measured coverage across that window was 17-24% of measurements against 62.6% on the target day, so each group's history came from a fifth of its traffic. A0 now extends its window by BASELINE_DAYS, which the SQL param and the lookback share so they cannot drift. In steady state the staleness join keeps this cheap; the wider window is what makes a cold start or a post-failure gap self-healing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.