Skip to content

Serve archived tracks for the map explorer - #696

Open
Volyarosh94 wants to merge 2 commits into
feat/mender-mirrorfrom
feat/archive-tracks-api
Open

Volyarosh94 wants to merge 2 commits into
feat/mender-mirrorfrom
feat/archive-tracks-api

Conversation

@Volyarosh94

@Volyarosh94 Volyarosh94 commented Oct 20, 2026 •

Copy link
Copy Markdown
Contributor

Based on #683 (Mender mirror), which sits on the rest of the open chain, so merge those first.

What

  • The tracks writer appends eight nullable columns after target_class: track_key, lane, pos_sigma_m, is_anomalous, anomaly_types, node_kind, solve_count and contributing_node_refs. Old files read with them null. Each flush file holds one row group per node kind, so availability can count per fleet from footers.
  • GET /api/data/tracks?from&to&bbox&fleet: a window of at most 60 minutes (ISO or epoch ms, UTC), an optional bbox (the antimeridian works), fleet real by default, synthetic or all. Up to 50,000 rows by frame time, with truncated past that. No session needed, like /data.
  • GET /api/data/tracks/availability?from&to&fleet: rows per hour over at most 92 days, from Parquet footers and file names only.
  • File work runs in the executor, with an LRU of decoded row groups (TRACKS_CACHE_MAX_BYTES, default 256 MiB).
  • services/node_kind.py holds the synthetic-id test that the writer and the reader share. It moved from tcp_handler.py, which imports it back.
  • architecture.md, .env.example and one staging smoke check.

Calls I made

  • I took the spec's defaults for the open questions. Rows with a private or probation contributor are dropped, not redacted. The explorer is a real-fleet feature and /sim stays live-only, though the API still takes synthetic and all. Times are UTC in the API. One year of compacted tracks is the retention target for phase 4.
  • contributing_node_refs isn't in the spec's table. On test and staging a mirrored node's ref only exists while the node is connected, so reading ids back after a restart lost those solves. The refs are now stamped when the solve is published. Today's private set still decides by id on every read. Older rows, and contributors that had no ref then, resolve at read time.
  • Ids are split on "," without trimming, the way the live feed looks them up, so a padded id drops its row instead of passing as a registered node.
  • A row's node_kind comes from the node's handshake when the solve is published, the same split the live real-only map uses. Mixed rows show only under fleet=all, and rows from before this change get node_kind from the id prefixes.
  • The writer skips a record whose position, time, node count or contributor ids can't be read, with one warning per flush, so a bad record can't block the batch or slip past the private-node filter.
  • The live feed and the archive decide which contributor refs are shown through one shared rule, node_refs.contributor_handle, and the solver reads owner identities from a cache that takes no lock.
  • A window also reads the next day's partition up to 10 minutes past to, for rows flushed just after midnight.
  • Availability counts are network-wide, so they can include rows the tracks route withholds. A row group counts in the hour of its earliest solve.
  • Known gap: the first 31-day availability call after a restart takes about 3.5 s on a test-sized history (1,440 raw files a day), and warm calls take about 25 ms. Phase 4's compaction fixes it. Rows are about 550 B of JSON each; Cloudflare compresses them on test-app.

Tested

just check green, new tests fail without their code, and a seeded month (520k rows a day) checked end to end with curl, plus a live fleet run writing the new columns.

Track archive rows carry the track key, lane, sigma, anomaly flags and node kind, one row group per kind.
/api/data/tracks returns a past window's solves under node refs and drops rows with private nodes.
/api/data/tracks/availability counts rows per hour from Parquet footers.
@claude

This comment has been minimized.

Skip rows without a real position, time or node count, and take node kind from the handshake.
Stamp refs without waiting on the registry, and name contributors by one rule in the live feed and the archive.
@claude

This comment has been minimized.

@claude

claude Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Claude finished @Volyarosh94's task in 2m 24s —— View job


Claude's review of PR #696 (re-run after "Harden the tracks archive after review")

Todo list:

  • Gather context (PR body, changed files, CLAUDE.md)
  • Diff the new commit (e410266) against the previous review's findings
  • Verify each fix against the code and its new tests
  • Post findings

The new commit, e410266 ("Harden the tracks archive after review"), is a direct response to the prior review on this PR. I checked each high-confidence finding against the diff:

# Prior finding Status
1 lat/lon missing → silently (0.0, 0.0), bypassing the privacy/validity filter Fixed. track_writer.py's new _invalid_fields requires a finite lat in [-90, 90] and lon in [-180, 180]; a record failing this is skipped (counted, one warning per flush) rather than written with a stand-in zero.
2 timestamp_ms/n_nodes missing → silently 0, sorting to the front of query_tracks and bumping legit rows off the 50k cap Fixed. Same _invalid_fields gate requires positive timestamp_ms, solve_ts_ms, n_nodes; _i() now also range-checks against the column's int32/int64 bounds.
3 tracks_store._published_refs re-derived the live feed's redaction rule instead of sharing it Fixed. New node_refs.contributor_handle() is the single rule; _published_refs and substitute_identities (the live feed) both call it now.
4 node_kind.is_synthetic_node (prefix-only) could disagree with core/state.py's node_world() (handshake-aware) Fixed. node_kind() now takes an is_synthetic callback; both solver append sites (stamp_archive_contributors in solver.py, used by known_lane.py too) pass state.node_world(n) == "sim". The archive reader still falls back to the prefix rule only for rows written before the column existed.
5 Solver hot path synchronously called node_refs.owner_identity() → registry lock/SQLite read on every cache miss Fixed. New node_refs.cached_owner_identity() reads _forward (already safely rebound as a whole dict, matching the existing ref_for pattern) with no _refresh() call and no lock. stamp_archive_contributors uses it at both append sites; new test test_solver_hook_does_not_wait_on_a_registry_refresh exercises this directly.
6 TRACKS_CACHE_MAX_BYTES=0 broke the "one warning per file version" contract (warn-once sentinel lived in the same zero-capacity cache) Fixed. _warn_once now tracks warned keys in its own bounded OrderedDict (_WARNED_MAX = 4096), independent of _cache.
7 test_failed_write_never_publishes_partial_parquet no longer exercised the default (non-grouped) write_parquet_batch path Addressed. New test_default_batch_is_one_write_table_call covers that path explicitly.

All six correctness/design findings and the one test-gap note from the prior review are resolved, each backed by a new, specific test (e.g. test_a_record_without_a_real_position_time_or_node_count_is_skipped, test_node_kind_is_the_stamped_verdict_else_the_id_prefixes, test_solver_hook_stamps_an_unloaded_registered_node_as_an_empty_slot). The remaining items from the prior review were explicitly marked as minor/non-blocking simplification suggestions (_Cache vs. snr_series's LRU, the _pairs() integer-encoding dedup trick, duplicated OLD_SCHEMA across two test files) and were left as-is, which is reasonable — none of them are correctness issues.

One small, genuinely new nit from this commit (not blocking):

  • tracks_store.py's availability tally was refactored from a 4-element list with magic indices into a proper _Tally NamedTuple (rows/real/synthetic/unproven) — this also happens to resolve the "magic indices" simplification note from the prior review, as a side effect of the hardening work rather than a direct response to it. No action needed, just noting it landed.

I was not able to execute just check or pytest in this sandbox (shell command execution for test runs required approval that wasn't available in this run), so this review is based on static reading of the diff and the new test names/bodies rather than a green test run. The PR body states just check was run green by the author.

Confirmed non-issues

  • CLAUDE.md compliance remains clean (.env.example, no /v1/nodes contract touched, uv.lock untouched, no secrets/hostnames).
  • The int32/int64 bound checks in _i() match the actual Arrow schema types (n_nodes/solve_count are int32, times are int64).
  • contributor_handle's validity rule ((recorded == node_id) == is_synthetic_node(node_id)) is logically equivalent to the old inline check it replaced in _published_refs, just shared now.

No blocking issues found in this round.
·

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.

1 participant