Skip to content

Preserve Ping Scores records and archived paths - #309

Merged
dborup merged 1 commit into
masterfrom
codex/ping-record-history-paths
Oct 6, 2026
Merged

dborup merged 1 commit into
masterfrom
codex/ping-record-history-paths

Conversation

@dborup

@dborup dborup commented Oct 6, 2026

Copy link
Copy Markdown
Owner

Summary

Keep historical Ping Scores distance records intact when the original GPS evidence disappears, and save map geometry for the current All-Time and This Week record cards so their View path can survive packet retention and a server restart.

Previously, a deep history sweep could replace a saved distance with a smaller value or no distance when the original endpoints no longer had GPS. Separately, View path depended entirely on retained raw packet observations, even though the record itself lived in the history sidecar.

Changes

  • Preserve a known historical distance when its original origin or farthest endpoint no longer has authoritative GPS. This includes an observer falling back to its IATA airport coordinates. Genuine coordinate corrections remain possible when the original endpoint coordinates are available.
  • Persist the distance origin separately from the current first hearer, so retaining a historical distance does not freeze first-hearer leaderboard credit.
  • Archive coherent path snapshots for the ten current record slots in the existing Ping Scores sidecar. Scores, archives and history metadata are published together. The v2-to-v3 migration does not scan or change the main packet database.
  • Add GET /api/ping-scores/{hash}/path?record=<slot>. Prefer the saved snapshot; require metric-coherent evidence before using a live fallback. Reapply current identity visibility rules to a copy of an archive and fail closed if current-name lookup fails.
  • Use this endpoint for record-card maps and copied/deep links. Mark saved geometry as archived, including its capture time and a position-age disclaimer. Show an honest unavailable message instead of a blank map when no usable geometry survives.
  • Leave the ordinary packet-path API and its default frontend links unchanged. A failed sidecar transaction commit is explicitly cleaned up before the single writer connection is reused.

Performance and bounds

The archive cache is bounded to ten slots. Each serialized snapshot is limited to 256 KiB, 128 branches and 4,096 points; oversized snapshots are not truncated into misleading paths. Existing cycle path results are reused, with at most one additional bulk lookup for the distinct missing record hashes, not one query per record.

A representative 20-branch capture benchmark on macOS arm64 / Apple M5 measured approximately 0.266 ms/op, 118,934 B/op and 915 allocations/op. This is a local measurement, not a worst-case latency guarantee or a speedup claim.

Verification

Fresh local verification on macOS arm64, Go 1.26.3:

  • Full cmd/server and cmd/ingestor Go suites: pass.
  • Focused history/path/migration tests with -race: pass; go vet ./... in the server module: pass.
  • sh test-all.sh: 222 test files passed, zero failed.
  • Ping Scores frontend tests: 19/19; Packet Path Map tests: 75/75.
  • Local Chromium browser matrix at 390×844, 768×1024 and 1440×900: 78 assertions passed, with real local archived-path API responses plus synthetic unavailable/initializing and navigation cases. Screenshots inspected; no browser exceptions.
  • Regression coverage includes lost GPS with both nil and smaller replacement distances, IATA fallback, genuine coordinate corrections, updated first-hearer credit, retention plus a real sidecar reopen, migration idempotence, real transaction-commit failure, current/saved/inactive visibility, fail-closed lookup, metric-coherent fallback, same-hash record navigation and copied links.
  • The independent reviewer approved the exact committed tree after the final migration/documentation delta.
  • JavaScript syntax, diff checks and the actual committed XSS diff preflight: pass.

The browser matrix is a local disposable probe, not a newly registered GitHub Actions E2E suite. Existing registered frontend tests were extended. GitHub Actions/Linux results are separate and are not claimed by these local runs.

Scope and limitations

  • Already-pruned route geometry cannot be reconstructed. Old records without a saved path show an explicit unavailable state. Likewise, a historical distance already lost before this change is not invented or restored from unrelated evidence.
  • Archives cover the ten current record slots, not permanent geometry for every historical ping. A link to a superseded slot can become unavailable.
  • Archived positions reflect capture time, not necessarily packet transmission time. Current visibility rules can remove an entire branch; no synthetic connection is drawn across a hidden relay.
  • The ordinary packet map's pre-existing rapid-close Leaflet animation edge case is unchanged. Historical-map initial framing is non-animated and the new browser cases pass.
  • Main-database schema, packet/ping retention, ingestor code, configuration, workflows and test registration are unchanged. The only migration and new writes are in the existing single-owner history sidecar.
  • No merge, staging/demo/production changes, deploy or publication performed. This PR is intentionally a draft with auto-merge off.

@dborup-agent

Copy link
Copy Markdown
Collaborator

Review — CS-MacBook PR#309 — head 8262c61

Dom: APPROVE with nits

Independent read-only review of the merged tree (git merge-tree origin/master 8262c61d = f7b3c2b6, clean merge against origin/master 22589f0). Evidence tags: [T] test, [A] architecture / source read, [K] ran live (binary or real HTTP). No changes were pushed; the PR was not merged, approved, or marked ready.

Findings

# Sev Area Finding
1 nit wording On a live (non-archived) path that exceeds the size ceilings, the handler returns reason:"archive_too_large" (ping_score_path.go:67). Accurate for archives, slightly misleading for a live path that was never archived. Cosmetic only; the enum is documented and the frontend maps it to a sensible message.
2 nit spec The OpenAPI/docs/api-spec.md entry documents the success envelope but not the {"error":"…"} 400/404/500 shape. Consistent with every other endpoint in this repo, so not a regression.

No correctness, privacy, scope, or read-only defects found.

Answers to the brief

  1. Read-only invariant (bug(db): vacuumOnStartup fails with SQLITE_BUSY when ingestor + server share DB (auto_vacuum migration #919 broken in single-container topology) Kpa-clawbot/CoreScope#1283). [A][T] The guard counts write-SQL string literals per non-test cmd/server/*.go; any file not in knownServerWriteSQL is allowed 0, so a write in an arbitrary other server file fails the test. The new ping_score_history_paths.go:4 entry maps to exactly four literals — ALTER TABLE …, CREATE TABLE IF NOT EXISTS ping_score_path_archives, DELETE FROM ping_score_path_archives, INSERT INTO ping_score_path_archives — all executed on the sidecar *sql.Tx (migration tx from migrateFrom→s.conn, and the cycle tx from s.conn.Begin()). None reference the shared meshcore.db, no ATTACH, no second handle. Mutant A [T]: adding INSERT INTO channel_proposals … to routes.go → TestServerSourceHasNoNewWriteSQL fails with routes.go:307:6: INSERT INTO, 0 allowed. Guard still catches new writes in any other server file.

  2. Sidecar migration v2→v3. [A][K]

    • Crash/commit safety: migrateFrom runs each step in its own tx and bumps _meta.schema_version inside that same tx; a mid-migration crash leaves the tx uncommitted → SQLite rolls back → re-run on next boot. applyPingScoreHistoryV3 is idempotent (guards ALTER behind pragma_table_info, CREATE TABLE IF NOT EXISTS). A failed Commit returns a *PingScoreHistoryMigrationError and closes the connection — no partial store. [T] TestPingScoreHistoryV2ToV3PreservesExistingScores, and the cycle-level commit failure is exercised by TestPingRecordArchiveCommitFailureLeavesLastPublishedAndPersistedState (deferred-FK trigger → real COMMIT failure → scores and old archive unchanged), which is exactly the new ROLLBACK/pool-retire cleanup path in UpsertDeleteAndMetadata.
    • Downgrade (ran it) [K]: I wrote a v3 sidecar with the PR binary (schema_version=3, distance_first_pubkey column + ping_score_path_archives table present), then opened the same file with a binary built from origin/master. Result: opens OK, readOnly=true, LoadAll returns the row reading the v2 column set (ignores the new column/table), a write attempt is rejected (…read-only (on-disk schema is newer…)), and schema_version stays 3 — never migrated down. This is the safe behavior for a stg rollback onto the same data-dir: the board serves frozen/read-only, no crash, no corruption, no data loss.
    • No main-DB access: [A] applyPingScoreHistoryV3 touches only the sidecar tx; no main-DB handle, no ATTACH, no scan. [T] …V2ToV3… asserts "migration fabricated old paths" is false.
  3. Distance preservation. [A][T]

    • Lost GPS / nil / smaller replacement → old FarthestKm/FarthestPubkey retained (preservePingDistanceWithoutGPS). IATA fallback is treated as not positioned (!o.iataFallback), so an airport fallback cannot shrink a record. Genuine correction stays possible only when both old endpoints are currently positioned by authoritative GPS.
    • First-hearer credit is kept separate from the distance origin via distanceFirstPubkey; first-hearer still updates the observer leaderboard. [T] TestPingRecordMissingGPSKeepsDistanceOriginNotFirstHearer, TestPingRecordDistanceAllowsGenuineGPSCorrection.
    • Untested edge I added & ran [T]: origin re-gains GPS while the farthest endpoint still lacks it. Verified across all four positioned-combinations — preserve unless both positioned; correction only when both; first-hearer never frozen. Passes.
    • Mutant B [T]: drop !o.iataFallback → TestPingRecordDistanceSurvivesKnownIATAFallback fails ("airport fallback shrank historical GPS distance"). Mutant C [T]: make preservePingDistanceWithoutGPS a no-op → TestPingRecordDistanceSurvivesMissingGPS + IATA test + my edge test all fail.
  4. Privacy / visibility. [A][T] filterPingScorePath re-applies current rules to a copy of the archive: identityHidden(cfg,pk,recordedName) || identityHidden(cfg,pk,currentNames…) — covering node blacklist, observer blacklist (fix(reach): hide blacklisted and hidden identities on the Reach page #68), and hidden-name prefixes on archived, current, and inactive names (privacy: node-404 checks only the newest observer alias, so an older hidden case-variant can leak name/role/timestamps #286/fix(server): hide node-404 identity if any observer alias is hidden (#286) #294 via hiddenIdentityNames). Whole hidden branches are dropped (no fictitious edge); a hidden first also nulls dependent relative metrics; approx neighbor-centroid geometry is stripped whenever any policy is active (contributors unidentifiable); airtime aggregate cleared when anything was filtered. A failed name lookup returns 500 (fails closed). A node hidden after archiving is removed on read. Notably the archived endpoint is stricter than the ordinary /api/packets/{hash}/path, which applies no identity filtering at the path layer — so there is no way for the archive to leak more than the live endpoint. [T] TestPingScorePathArchivePrivacyAndImmutability (6 cases incl. inactive-node), TestPingScorePathVisibilityLookupFailsClosed (DROP observers → 500, no leak), TestPingScorePathApproximatePrivateContributorsNotExposed.

  5. New endpoint GET /api/ping-scores/{hash}/path?record=<slot>. [A][T][K] record validated against the fixed allTime/thisWeek × 5-kind set (rejects __proto__); hash must equal the slot's current record. Ran live on a real server (port 13900, migrated e2e-fixture.db): invalid/missing record → 400 JSON {"error:"invalid ping record slot"} (application/json), __proto__ slot → 400 JSON, valid record + unknown hash → 404 JSON, while a non-API route returns SPA HTML — i.e. API errors are JSON, not SPA (fix(server): JSON 404/405 for unknown /api paths and wrong methods #266). Uninitialized board → 200 {"status":"initializing"} [T]. Response types are named structs (PingScorePathResponse), no new map[string]interface{} in any changed non-test Go file. OpenAPI schema + docs/api-spec.md match the envelope (status/capturedAt/reason/path).

  6. Bounds & performance. [A][T] Ceilings 10 slots / 256 KiB / 128 branches / 4096 points. Over-limit paths are not archived and never truncated (boundedPingPathJSON errors → slot keeps old evidence or none; live over-limit → unavailable). DB-level CHECK(length ≤ 256KiB) + LoadPathArchives rejects oversized/row-count overflow before parse. [T] TestPingRecordPathArchiveBoundsAndOversizedLoad, TestPingRecordPathArchiveSlotBoundAndInvalidWriteRollback. One single extra GetPacketPathsBulk for the distinct missing hashes (≤10), not per-record [T] TestPingRecordPathArchivesDeduplicateExtraBulkFetch. Capture/marshal happen before the tx (no expensive work under a lock; pingScores is an atomic pointer). Scores + archives + integrity/gap/metadata publish in one transaction (upsertDeleteMetadataAndArchives).

  7. Frontend. [T] test-ping-scores.js 19/19, test-packet-path-map.js 75/75, test-frontend-helpers.js 709/709 (merged tree). XSS: scripts/check-xss-sinks.sh --diff equivalent on both changed public files = clean, and --file (whole-file) scan clean; the only new interpolation (data-view-record) is a fixed server-side constant, archive note/disclaimer use textContent. No hardcoded colors in added lines (all var(--…)). Deep links / copied links (shareURL, mapRoute), two-slots-same-hash navigation, retry, Back/Forward closed-entry lifecycle, "unavailable"/"initializing" messages instead of a blank map, and __proto__-reason guarding are all covered. Ordinary packet-path map is unchanged for non-historical callers (animation defaults preserved; the new activeMap===map/requestGeneration guards are behavior-safe fixes). I did not drive a real Chromium against the page (see "Not verified").

  8. Scope. [A] Exactly 1 commit, author dborup <kontakt@meshview.dk>, subject "Preserve ping record distances and archived paths". All changed files are under cmd/server/, docs/, public/, test-*.js. No ingestor, workflow, config, .github, or main-DB schema changes. No closing keywords in the PR body. The "independent reviewer approved" line in the description is not part of our process and was disregarded.

Tests run (all green, merged tree vs origin/master 22589f0)

Mutants (mine)

  • A [T] write-SQL in routes.go → read-only guard fails with file:line. ✔ caught.
  • B [T] drop !o.iataFallback → IATA-preservation test fails. ✔ caught.
  • C [T] disable preservePingDistanceWithoutGPS → missing-GPS + IATA + my edge test fail. ✔ caught.
  • Plus my own 4-combination edge-case unit test (origin re-gains GPS / farthest still missing) — passes on the real code.

Not verified

  • A real headless-browser render of the Ping Scores page against the live server (the PR adds no new registered GH-Actions E2E — by design it extended the existing jsdom harnesses, which I ran green; frontend behavior is otherwise covered by those + live API probes).
  • The initializing state over live HTTP (timing-sensitive at boot; covered by TestPingScorePathInitializingIsNotNoPings).
  • End-to-end CI completion (ingestor/JS/lint/E2E jobs were still running when I posted this).

Overall: a tightly-scoped, exceptionally well-tested change. Read-only, privacy, downgrade, bounds, and scope invariants all hold under code review, the suites, three of my own mutants, and live runs. The two nits are cosmetic and non-blocking.

@dborup
dborup marked this pull request as ready for review October 6, 2026 08:33
@dborup
dborup merged commit 5a8b113 into master Oct 6, 2026
6 checks passed
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.

2 participants