feat: contribute 1.7.13 -> 1.7.14 (kinematic speed gate against FMDN teleport fixes, round-trip recovery, manual-locate log redaction, branch-neutral release automation)#205
Conversation
…l-locate diagnostic (#1193) * fix(nova): redact device name and drop misleading cause list in manual-locate diagnostic The `not fcm_token` branch in `get_location_data_for_device` logged "Failed to get FCM token for %s: ... (client/token unavailable or no coordinator)" at ERROR with the raw device name. This had two defects: 1. R6 redaction (AGENTS.md Section 5): the raw device display name (e.g. "samsung SM-S938B") leaked into a user-facing ERROR record. The rest of this module already keeps such names at DEBUG only (see the outer surfacing handler in the same function). 2. Misleading diagnostics: the hard-coded closed set of three causes was incomplete. `async_register_for_location_updates` fails closed on several paths (missing canonical id, no coordinator, unavailable client/token, and a fail-closed reconnect that never reached STARTED), and each already logs its own specific WARNING immediately before returning falsy. When the real cause was the reconnect/STARTED timeout, the enumerated list pointed readers at the wrong thing. The branch now defers to that authoritative preceding WARNING, drops the misleading enumeration, keeps the device name at DEBUG only, and reports at WARNING instead of ERROR (the outcome is fail-closed and self-heals on the next poll, so a second ERROR overstated the severity). Adds a regression test that fails on the old behavior (name at ERROR, enumeration present) and passes with the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * style(test): apply ruff format to regression test Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…s (R6) (#1194) Completes the R6 redaction started in PR #1193. Every remaining user-facing (>= WARNING) record in `location_request.py` logged the raw device display name via a `... for %s` argument, in violation of R6 (AGENTS.md Section 5: redact derived identifiers such as device names from logs). Each such record now states the failure and its cause without the name and keeps the raw name at DEBUG only, mirroring the module's established Count@WARNING / Name@DEBUG pattern (PR #1129 / #1193). Swept sites (16): `get_location_data_for_device` - FCM registration failure (ERROR) - Nova request ladder: rate-limit, server/HTTP, auth, network/client, generic (WARNING/WARNING/WARNING/WARNING/ERROR) - unexpected-device mismatch (WARNING) - FCM unregister failure in `finally` (WARNING) `_make_location_callback` - decoder/decrypt import and attribute failures (ERROR) - protobuf parse failure (ERROR) - per-device decrypt failure (ERROR for session/auth, WARNING for per-device) - unexpected async-decryption error (ERROR) - empty-after-decrypt (WARNING) - outer callback failure (ERROR) The two sites beyond PR #1193's enumerated follow-up list (NovaHTTPError server error, unexpected-device mismatch) are included because they are the same R6 class; a partial sweep would leave the class inconsistent. Visible message prefixes are preserved so existing severity/diagnostic tests (e.g. the "Failed to process location data" filter) keep matching. Adds a dedicated regression suite that is RED on the pre-fix code (sentinel device name present in a >= WARNING record) and GREEN with the redaction, covering both the caller ladder and the callback branches. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* Add kinematic speed gate to reject FMDN teleport fixes (#177) * i18n: sync speed_gate_enabled option into all locales The translations validation gate (sync_translations.py --check) requires the new speed_gate_enabled option key to be present in every locale. Populate the eight remaining locales (es, fr, he, it, nl, pl, pt-BR, pt) via the canonical script/sync_translations.py, which seeds the English source text as a fallback until translated. * fix(cache): let accuracy-less crowd fixes bypass the speed gate The kinematic speed gate hard-dropped a far crowd fix via return False before the downstream _is_significant_update sanitization ran. For an accuracy-less fix (accuracy=None) that sanitization is what flags accuracy_estimated=True, so dropping it regressed the last-good protection covered by test_plus_code_last_known.py (KeyError: 'accuracy_estimated'). An accuracy-less crowd report is not a clean teleport discriminant anyway: the decoder strips the accuracy number (acc_rank=-inf) and the reliable last-good is already protected by is_reliable_fix, independent of this gate. Guard the gate with 'new_acc_raw is not None' so accuracy-less fixes fall through. Add a mutation-guard test (accuracy-less crowd fix at impossible speed -> passes, no reject counter). Real crowd teleports carry a measured accuracy and stay gated. * fix(cache): treat invalid accuracies as accuracy-less before speed gating Codex review of 5bfeafd: the #1181 bypass guarded only the missing-key case (new_acc_raw is None). A crowd fix carrying Android's no-accuracy sentinel (accuracy=0.0) or any sub-physical/non-finite value coerces to a non-None float, so the gate could still hard-drop it via `return False` before _is_significant_update sanitizes it into accuracy_estimated=True, regressing the #1181 last-good protection for that class of report. Widen the guard to the same error-code set that _safe_accuracy() maps to the 200m fallback: gate only a fix whose accuracy is present, finite and >= MIN_PHYSICAL_ACCURACY_M. Reuses the already-imported math and MIN_PHYSICAL_ACCURACY_M SSOT; no new private helper, no status_code duplication. Add regression tests 21-22 pinning the sentinel (0.0) and sub-physical (0.0005) cases fall through the gate and leave the reject counter untouched. * fix(cache): key own-report gate bypass off cryptographic provenance Codex review of 74600c2: the own-report bypass read the raw `is_own_report` flag. A network/foreign report can carry a spurious server-supplied `is_own_report=True` - this integration's own crowdsourced uploader stamps network reports that way while they carry a non-empty publicKeyRandom (decrypt_locations.py:1991-1998). Keying the bypass off the raw flag alone would let such a spoofed own-flag skip the speed gate, so an impossible teleport from that crowd report would still be accepted. The decrypt layer already hardens is_own_report = is_own_report and not is_network_report (decrypted_location.py:39). Mirror that invariant at the gate so the bypass uses cryptographic provenance on every path: bypass only when is_own_report is set AND is_network_report is falsy. An absent is_network_report is treated as not-network (genuine own reports never carry the network flag), so real own fixes keep bypassing. Add regression tests 23-24: a spoofed network report (both flags set) at teleport speed is now gated and counted, while a genuine own report with no network flag still bypasses.
…1197) Net content delta of the round-trip escape hatch (#1196) rebuilt as a single commit on top of 1.7. The kinematic speed gate (#1195) is already in 1.7 via squash-merge, so its content nets to zero here; this commit carries only the round-trip additions (+538/-0, 19 files). Rebuild avoids the squash-merge divergence that made a direct merge of feature/speed-gate-teleport conflict against 1.7. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…1196 follow-up) (#1198) * fix(cache): round-trip anchor lifecycle + symmetric speed-gate guard (#1196 follow-up) Three coherent fixes in the round-trip / speed-gate region of cache.py, all against the 1.7 baseline that now carries the stateful round-trip escape hatch (#1197): - F-FABLE-1: guard the anchor housekeeping pop on ``delta_anchor >= 0``. A delayed out-of-order report with last_seen between anchor A and B yields delta_anchor < 0; popping there would destroy the anchor and strand a later genuine return. Only house-keep forward. - F-FABLE-2: invalidate a stale round-trip anchor after a trusted own-report clear-jump commits. Detection sets a slot marker in the clear-jump branch; the marker is popped early (before sanitize/merge, no cache leak) and the anchor is popped only AFTER the payload commits and passes the significance gate, so a rejected payload never triggers it (F-CODEX-7 semantics preserved by placement). The polling test-double fallback strips the marker too. - F-CODEX-1 residual: extend the speed gate with ``is_reliable_fix(existing)`` so it fires only when BOTH endpoints carry real measured accuracy. Gating a fix against an estimated 200 m fallback endpoint would reject it against a phantom position and strand the tracker on the estimate. Tests: 12 new cases across round-trip and speed-gate, each with a mutation counter-proof (reverting the fix turns the paired test red). Known bounded trade-offs (F-1/F-2/F-3, all TTL/radius-bounded) are documented in-code and deferred to a follow-up. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(cache): clear round-trip anchors of propagated shared devices (F-CODEX-9) A trusted own-report clear jump popped the round-trip anchor only for the source device_id. update_device_cache then propagates the same fresh trusted location to sibling device ids that share the identity_key (_propagate_location_to_shared_devices), leaving their _round_trip_anchors stale. A later crowd echo near the old anchor for a propagated target could take the round-trip recovery path and roll that target back to the stale position, even though the trusted location was already propagated there. Fix (rides the propagation data-flow, SA-8d): - _propagate_location_to_shared_devices gains supersede_anchors (default False); inside the loop, after a target adopts the fresh location (past the freshness gate), its stale anchor is popped too. A fresher target that skips propagation keeps its own anchor. - The supersede marker is now set on any trusted own clear-jump, not only when the source holds an anchor: a propagated sibling may hold a stale anchor even when the source does not. All anchor pops are idempotent, so marking without a source anchor is a harmless no-op on the source side. Tests: shared-device invalidation, fresher-target preservation (scoping), source-without-anchor sibling clear, and the rewritten fusion-marker contract; each with a mutation guard. Behavioral proofs kept within ROUND_TRIP_TTL_S so they are live, not TTL-vacuous. --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* ci: branch-neutral semantic-release + three-file version sync
Replace the hard-wired branch = "main" with a [tool.semantic_release.branches.release]
regex that matches main and any X.Y maintenance line, add tag_format = "{version}"
to anchor on the recent tag convention, and add version_variables so const.py and
manifest.json are stamped alongside pyproject.toml. Drop the INTEGRATION_VERSION type
annotation (the version_variables regex cannot match NAME: str = "x").
Update the two source-parsing guard tests (test_hacs_validation, test_hypothesis_properties)
to the annotation-free form; they now also guard against re-introducing the annotation.
Make release.yml trigger workflow_dispatch-only (runtime branch choice, no hard-wired name).
* ci: draft-handshake release stamping (protection-aware)
Workflow A (release.yml) now only PROPOSES the next version via
`semantic-release version --print` and opens a DRAFT release pinned to the
dispatched branch (--target); the one-shot version+publish and inline ZIP
steps are removed. Nothing releases automatically.
Workflow B (release-stamp.yml, new) fires on release:published, stamps the
exact published tag into the three version literals via script/stamp_version.py
(new; deterministic, stdlib-only; exact-one-hit assertion per file; tag-format
validation X.Y.Z[bN][.N], no `v` prefix), commits with [skip ci], pushes
protection-aware (direct push; on a 403/protected-branch rejection only, falls
back to a short-lived PR + auto-merge, dormant today), and rebuilds+uploads the
HACS ZIP from the stamped tree so the installed asset carries the right number.
The stamp mirrors the maintainer-chosen tag verbatim (honours hand-picked and
beta `bN` tags), so beta/final isolation across forks needs no PSR prerelease
config. All event-derived values (tag_name, target_commitish) reach shell only
via env indirection, never `${{ }}` in run: blocks.
Also fixes a latent dry-run bug: python-semantic-release v10 requires the
global --noop BEFORE the subcommand (`version --noop` / `changelog --noop` are
not valid options).
Refs #177, #1199
* chore(ci): retrigger checks
* ci: honest version proposal + stamp regression tests (Codex #1199)
Workflow A (release.yml) now detects branch-name-agnostically when
semantic-release cannot propose for a line (PEP 440 maintenance tags it
cannot parse) by comparing its --print-last-released view with the real
topological line tip (git describe), and opens an empty hand-tag draft
instead of a stale number. It also aborts honestly when no release is
due (proposal equals the last released version) instead of drafting an
already-released tag.
Add tests/test_stamp_version.py: VERSION_RE against the real tag matrix
(SemVer + PEP 440 betas/four-segment), the three per-file stampers
(table/key scoping + exact-one-hit), and a byte-parity lock between
VERSION_RE and the mirrored shell guard in release-stamp.yml.
Fix stamp_const: the greedy trailing whitespace match deleted the blank
line after INTEGRATION_VERSION on every stamp; restrict it to horizontal
whitespace so the surrounding layout is preserved.
Addresses the two Codex P2 findings on #1199 (no-release proposal guard;
PEP 440 tags vs. SemVer-only proposal).
* ci: HACS zip_release packaging + PEP 440 tag order (Codex #1199)
Option A (HACS packaging): set zip_release=true + filename in hacs.json and
build the release ZIP from the domain *contents* (cd into the domain dir, zip
`.`) so manifest.json sits at the ZIP root. HACS extracts the asset directly
into custom_components/googlefindmy/; the old parent-dir zip nested it one
level too deep. Without this HACS installed the git source archive and ignored
the stamped ZIP, so the version stamp never reached users.
F-NEW-2 (tag regex): reorder VERSION_RE and the byte-parity shell guard to
(\.N)?([ab]N)? so the numeric release tuple precedes the prerelease per PEP
440 -- now accepts 1.7.14.1b1 and rejects 1.7.14b1.1 (both previously wrong).
Tests: add a ZIP-structure regression guard + zip_release/filename coupling
check, and the two boundary tags to the VERSION_RE matrix.
* fix(release): stamp from the published tag, resolve owning branch robustly
release-stamp.yml checked out github.event.release.target_commitish, which
GitHub documents as unused once the git tag already exists (it then defaults
to the repo default branch). For a release from a pre-existing hand tag on a
maintenance line, that stamped and packaged the wrong tree. The bug was masked
on jleinenbach only because its default branch happens to be the 1.7 line; it
would fire on the planned BSkando transfer (different default branch).
- Check out github.event.release.tag_name (the exact published commit) so the
stamped tree and the uploaded HACS ZIP always match the release.
- Resolve the stamp-push target branch from the tag commit itself
(git branch -r --contains), honoring target_commitish only when it is a real
owning branch; push fast-forward only and skip gracefully (::notice::) when
the tag is behind the branch tip -- never force-push a maintenance branch.
- Order the push error classification protected-before-non-ff (a protected
rejection also contains 'rejected').
- Drop the now-obsolete SHA exit-1 guard (it aborted before the ZIP upload and
the push target is no longer taken raw from target_commitish).
- Add a regression guard test pinning the tag_name checkout and the branch
resolution (CA-WORKFLOW-EVENT-PAYLOAD-CONTRACT-001).
* fix(release): drop stale target_commitish tiebreaker in branch resolution
The owning-branch resolution honored the stale release.target_commitish
hint before the single-candidate check. When the published tag sits on
several branches (e.g. main + 1.7 on a non-diverged maintenance line),
the stale default-branch hint was among the candidates and got picked,
stamping the wrong branch. GitHub documents target_commitish as unused
once the tag already exists (it defaults to the repo default branch).
Remove the hint tiebreaker entirely and the now-dead TARGET_BRANCH env:
stamp only when exactly one branch owns the tag, otherwise skip safely
(the HACS ZIP asset already carries the version). This heals both the
tie case (ambiguous -> safe skip) and the diverged case (single owner ->
correct), and removes the failure source instead of adding a heuristic.
Tighten the workflow guard test to assert TARGET_BRANCH is gone, closing
the gap the prior 'git branch -r --contains present' check left open.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4128f7a46f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| anchors[device_id] = { | ||
| "lat": existing_lat, | ||
| "lon": existing_lon, | ||
| "ts": new_ts, | ||
| } |
There was a problem hiding this comment.
When an incoming crowd fix has an invalid future last_seen, _apply_weighted_location_fusion() runs before _is_significant_update() in update_device_cache(), so this assignment can create a round-trip anchor for a payload that is rejected a few lines later and never committed. If the tracker subsequently moves away, that phantom anchor can still recover a later crowd echo back to the old position even though the A→B jump that created it was discarded; mirror the supersede handling and defer anchor creation/consumption until after significance validation and cache commit.
Useful? React with 👍 / 👎.
| if ! gh release create "$PLACEHOLDER" --draft --target "$REF_NAME" \ | ||
| --generate-notes \ | ||
| --title "DRAFT - replace tag with your version (e.g. 1.7.14 or 1.7.14b1) before publishing"; then |
There was a problem hiding this comment.
In hand-tag mode this still creates a real set-release-tag git tag as soon as the draft is opened: the gh release create docs state that if the tag does not exist, it is automatically created and --target selects its branch. Because release-stamp.yml only validates the tag on publish, a saved/deleted/edited draft can leave this invalid tag on the maintenance branch, where git describe --tags can later see it as the line tip and keep routing future releases into the hand-tag path or collide with the placeholder. Use a real version tag or another draft mechanism that does not create a placeholder tag.
Useful? React with 👍 / 👎.
…Fund A) + release-tip VERSION_RE guard (Fund B) (#1200) * fix(cache): defer round-trip anchor seed/consume to post-commit (Fund A) The round-trip anchor store was seeded (accept path) and consumed (recovery path) inside _apply_weighted_location_fusion, before the significance gate and before the payload commits. A payload the significance gate later rejects (early return) then left a phantom anchor (seed) or a prematurely burnt anchor (consume, the symmetric D1 case) behind. Defer both mutations, mirroring the existing _supersede_round_trip_anchor pattern: the fusion records the intent as a marker on new_data (_round_trip_anchor_seed / _round_trip_anchor_consume); update_device_cache pops the markers early (before sanitize/merge, so they never leak into the cached row) and applies them only after the commit and the significance gate. A rejected payload thus never touches the anchor store; F-CODEX-7 semantics hold automatically via the early return. The preapply paths (locate.py, polling.py) share the same slot object through to update_device_cache, so the marker survives without extra plumbing; the polling test-double fallback strips the markers for hygiene. Tests: the existing round-trip suite exercised the fusion unbound, so the fusion-then-significance ordering was never driven and no test could catch this. New integration tests drive update_device_cache for both axes (seed + consume) with the ordering invariant, a positive control, and a re-runnable mutation cross-check; the existing unbound-fusion tests now assert the intent marker on new_data. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * ci(release): gate REAL_TIP against VERSION_RE (Fund B hardening) Codex flagged gh release create --draft as leaving a real git tag that git describe --tags would misread. A live non-destructive test on this fork refuted the premise: a --draft release creates no git tag (git ls-remote --tags empty, the tag ref returns Not Found, GitHub stores an untagged-... draft). No code fix of the alleged path is made. Independently, gate REAL_TIP (git describe --tags) against the canonical PEP 440 VERSION_RE (SSOT script/stamp_version.py, byte-mirrored in release-stamp.yml), so a foreign non-version line tip (e.g. a stray set-release-tag, nightly, v2) can no longer poison the PSR routing comparison and spuriously open a hand-tag draft. PEP 440 beta/four-segment tags still match VERSION_RE, so the intended MODE=hand protection is preserved. Extend the byte-parity guard test to cover both workflows, plus a deterministic gate-behaviour test (foreign tip blanked, PEP 440 tip kept). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7756ee7ca3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self._propagate_location_to_shared_devices( | ||
| device_id, slot, supersede_anchors=supersede_anchor | ||
| ) |
There was a problem hiding this comment.
When two device ids share the same identity_key, an accepted A→B jump on one id is propagated to the sibling by the call below, but the new anchor_seed is only stored for the source device_id just above. If the next B→A return report arrives under the propagated sibling id, the speed gate will find no anchor for that id and reject the legitimate round trip, leaving the shared tracker stranded at B. Pass the seed through the propagation path for targets that actually adopt the fresh B row.
Useful? React with 👍 / 👎.
| if ! gh release create "$PROPOSED" --draft --target "$REF_NAME" \ | ||
| --generate-notes --title "$PROPOSED"; then |
There was a problem hiding this comment.
This draft step is not tag-neutral: I checked the GitHub CLI manual, and gh release create automatically creates a missing git tag, with --target only choosing where that tag points (https://cli.github.com/manual/gh_release_create). If a maintainer uses this new proposal flow and then abandons/deletes the draft instead of publishing it, the release-stamp workflow never runs but the version tag already exists, so later release calculation can treat the version as released while the three stamped files remain stale.
Useful? React with 👍 / 👎.
… A) (#205) (#1201) When two device ids share one physical tracker (same identity_key), an accepted A->B jump seeds a round-trip anchor at the pre-jump position A, but only for the source device_id. _propagate_location_to_shared_devices adopts the fresh B row for every sibling and (since #1198/F-CODEX-9) propagates the supersede pop, but never the seed. The recovery read keys hard on device_id, so a later B->A return arriving under a sibling id finds no anchor, the speed gate rejects it as a teleport, and the shared tracker is stranded at B. Propagate the seed on the same data-flow, mirroring the F-CODEX-9 supersede block: a new seed_anchors parameter seeds each adopting sibling (past the freshness gate) with an independent copy of the source-A anchor. No lazy-init is needed in the propagation block: a non-None seed_anchors means the source post-commit seed block already initialized the store. SA-8 breadth sweep: the recovery consume pop is deliberately NOT propagated (a B->A return on one sibling must not delete another sibling's still-legitimate A-anchor). Only seed and supersede are propagation-symmetric; they are mutually exclusive per report (accept-path seed vs own-report supersede), so no target is double-mutated. Tests: a mutation-sharp integration test (A->B accept seeds the sibling, B->A return under the sibling recovers) and a scoping test (a fresher sibling that skips the freshness gate does not inherit the seed), each with a re-runnable mutation counter-proof. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Codex answers for #205 (reviewed commit 7756ee7)Finding A (cache.py, propagate round-trip anchors to shared trackers): real, fixedConfirmed and fixed. The source seeds Fix (PR follows): a new SA-8 design note beyond the flagged line: the recovery consume pop is deliberately NOT propagated, because a return on one sibling would delete another sibling's still-legitimate A-anchor. Only seed and supersede are propagation-symmetric; they are mutually exclusive per report, so no target is double-mutated. New mutation-sharp integration test + scoping test cover both the propagation and the fresher-sibling skip. Finding B (release.yml, avoid creating tags for draft-only releases): false positiveThe premise that
So a draft release does not create a git tag; Independently of the (refuted) draft-tag path, |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a6bd544c39
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if delta_anchor >= 0: | ||
| anchors.pop(device_id, None) |
There was a problem hiding this comment.
When an active A->B round-trip anchor receives a different rejected crowd fix C within the TTL but outside the return radius, delta_anchor >= 0 is true, so this deletes the anchor even though it has not expired. A later genuine B->A return within the original 15-minute window then has no anchor and is speed-gated, defeating the new recovery feature whenever a noisy report arrives between the trip legs. Please only expire the anchor when the TTL has elapsed (or otherwise leave it in place for non-return rejects).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Finding: "Preserve anchors on unrelated rejected fixes" — CONFIRMED, fixed.
You are right. In the speed-gate reject branch, the housekeeping pop guarded the
deletion on delta_anchor >= 0, which conflated two distinct cases:
- a genuine TTL expiry (
delta_anchor > ROUND_TRIP_TTL_S), and - an unrelated reject that is still inside the TTL window but landed outside the
return radius (0 <= delta_anchor <= ROUND_TRIP_TTL_Sand
return_dist > ROUND_TRIP_ANCHOR_RADIUS_M).
The second case is exactly the noisy-report-between-the-trip-legs scenario you
describe: a still-valid A->B anchor was dropped, so a later genuine B->A return
within the original 15-minute window found no anchor and was speed-gated,
defeating the recovery feature.
Fix: the housekeeping pop now fires only on a genuine TTL expiry
(delta_anchor > ROUND_TRIP_TTL_S). A within-TTL reject outside the radius, and
the previously handled out-of-order case (delta_anchor < 0), both leave the
anchor in place; it expires naturally once the TTL elapses. Recovery stays
one-shot and radius-gated, so keeping the anchor longer cannot open a ping-pong
(DF-1 bulwark unchanged).
Covered by a new behavioral test: an active anchor survives a within-TTL
out-of-radius reject, and a subsequent genuine near-A return then recovers
(mutation check: reverting the guard turns the test red). Genuine-expiry pop and
the out-of-order preservation keep their existing tests green.
Fixed on jleinenbach:1.7 in PR #1202.
…r rejects (#1202) The speed-gate reject branch house-kept the round-trip anchor whenever delta_anchor >= 0, conflating a genuine TTL expiry with an unrelated crowd fix that landed within the TTL window but outside the return radius. A noisy report between the trip legs therefore dropped a still-valid A->B anchor, so a later genuine B->A return within the original 15-minute window found no anchor, was speed-gated, and the recovery feature was defeated. Gate the housekeeping pop on a genuine TTL expiry (delta_anchor > ROUND_TRIP_TTL_S). A within-TTL far reject and an out-of-order report (delta_anchor < 0) both leave the anchor in place; it expires naturally by TTL. The single "> TTL" test subsumes the old F-FABLE-1 ">= 0" guard. Recovery stays double-gated (return_dist <= radius AND delta_anchor <= TTL) and one-shot, so keeping the anchor longer cannot open a ping-pong. Add two behavioral tests (within-TTL far reject preserves the anchor; subsequent genuine return recovers), each with a re-runnable mutation check. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9880e5ddaa
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # anchor: | ||
| # custom_components/googlefindmy/AGENTS.md ("Version bump touches three files"). | ||
| version = "1.7.13" | ||
| version = "1.7.14.0" |
There was a problem hiding this comment.
The new release workflow still invokes poetry run semantic-release version --print[-last-released], but this stamps [tool.poetry] with a four-segment PEP 440 version. I checked the Python Semantic Release 10 docs, which state that the configured project version must be SemVer MAJOR.MINOR.PATCH and that PEP 440 values are not supported; with 1.7.14.0, the PSR commands in .github/workflows/release.yml fail or get suppressed into the hand-tag path, so branch-neutral automatic proposals/dry-runs are broken after this release. Keep the Poetry value SemVer-compatible or avoid running PSR against PEP-440-stamped trees.
Useful? React with 👍 / 👎.
… lines (#1203) The coupled-triple comment claimed semantic-release keeps all three version literals in sync automatically on any release line (main and every X.Y maintenance branch) and that a manual bump is only a fallback. That contradicts .github/workflows/release.yml (semantic-release only understands SemVer; PEP 440 maintenance lines, tagged four-segment .N or beta bN, are silently skipped by PSR, so the maintainer hand-tags there). Rewrite the comment to match the workflow: PSR auto-sync only on SemVer lines (main); on PEP 440 maintenance lines the hand-tag + release-stamp.yml stamp (script/stamp_version.py) is the primary path, not a fallback. Comment-only change; the version literal and all [tool.*] settings are unchanged. Addresses the Codex review finding on #205. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 22748127c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if ( | ||
| self._speed_gate_enabled() | ||
| and not incoming_is_own | ||
| and new_acc_measured | ||
| and is_reliable_fix(existing) | ||
| ): |
There was a problem hiding this comment.
When a user restores a legacy/corrupt cached row with Android's no-accuracy sentinel (for example accuracy: 0.0, or another sub-physical/non-finite value) and no accuracy_estimated flag, this guard still treats the cached endpoint as reliable because is_reliable_fix() only requires accuracy is not None and not estimated. A later valid measured crowd fix after a far move can then be rejected by the new speed gate against that unreliable baseline, preventing the existing self-healing path from accepting the new position; please apply the same finite/>= MIN_PHYSICAL_ACCURACY_M measured-accuracy check to the cached endpoint before gating or seeding anchors.
Useful? React with 👍 / 👎.
…_fix (Codex #205) (#1204) * fix(coordinator): require finite sub-physical accuracy in is_reliable_fix is_reliable_fix() only checked accuracy presence + not-estimated, so a cached/reference endpoint carrying Android's 0.0 no-accuracy sentinel (or a negative, sub-MIN_PHYSICAL_ACCURACY_M, or non-finite value) WITHOUT the accuracy_estimated flag was treated as a real, reliable fix. The incoming side already filters this exact error-code set (math.isfinite + >= MIN_PHYSICAL_ACCURACY_M), so a genuine measured crowd fix could be speed-gated against a 0 m phantom reference and dropped (Codex #205, #1179 poisoning class). The predicate's own docstring ("only a real measurement") and the gate comment ("BOTH endpoints carry real measured accuracy") were not backed by the code. Fix at the predicate level (DRY): is_reliable_fix now also requires a finite, >= MIN_PHYSICAL_ACCURACY_M, non-bool numeric accuracy, mirroring the incoming-side new_acc_measured check. This closes the gap at every call site in one place (speed gate, round-trip anchor seed source A, last-good retention, both device_tracker last-good sites). has_usable_accuracy (the display gate) stays lenient so a sanitized estimated/0.0 fix is still shown. Consistent with the F-2 trade-off: an unreliable reference makes the gate block fall through rather than gating against a phantom reference. Four mutation-sharp tests: the predicate rejects 0.0/-1.0/inf/nan/bool; the speed gate does not fire against a 0.0 cached endpoint; the retention does not advance a 0.0 last-good over a reliable one; a 0.0 anchor source A seeds no anchor. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * test(coordinator): pin >= floor boundary in is_reliable_fix (Codex #205 review followup) Add a boundary assertion for the MIN_PHYSICAL_ACCURACY_M floor: exactly MIN_PHYSICAL_ACCURACY_M is reliable, a hair below is not. Closes a mutation hole flagged by the terminal diff review (a >=/> mutation on the floor check would otherwise go undetected). Verified mutation-sharp locally (>= -> > turns the boundary test red). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Summary
This contributes the next slice of the
jleinenbachfork's1.7line (integration 1.7.13 -> 1.7.14) on top of the already-merged1.7.13contribution (#204). It bundles 6 fork PRs plus one automated release commit: a headline kinematic speed gate that rejects physically impossible crowd-sourced "teleport" fixes, a stateful round-trip recovery so a legitimate there-and-back trip is not lost to that gate, a manual-locate log privacy fix, and a branch-neutral release automation for the release/version-stamp toolchain.GitHub shows a clean 7-commit (plus one catch-up merge), 34-file diff (
+2986 / -97). That line count is misleading: the runtime code change is small (custom_components/**/*.py= 7 files, +432 / -53). The bulk is new tests (tests/**= 9 files, +1999 / -3) and the release/CI toolchain (.github/**+pyproject.toml+script/**= 4 files, +497 / -33); the i18n/package strings are 12 files, +46 / -0 (two new option labels across ten locales). Nothing shipped to Home Assistant users depends on the CI toolchain.This becomes the BSkando 1.7.14 release; the tag/release itself is a separate next step. The version literal already present in the code (
const.py/manifest.json/pyproject.toml) is1.7.14.0, the four-part PEP 440 form the new release automation produces (see the reviewer note under Changed).Motivation
Since
1.7.13, the crowd-sourced location path needed hardening against the Find My Device Network's "teleport" failure mode, plus two smaller fixes worth contributing together. In plain terms:Change Log (human-readable)
Added
speed_gate_enabled("Reject impossible location jumps"). When a crowd fix would require a physically impossible point-to-point speed to be reached from the previous accepted position, it is rejected instead of teleporting the tracker. The gate is an absolute physical cap (DEFAULT_MAX_PLAUSIBLE_SPEED_MPS = 400.0, ~1440 km/h), not relative to your usual speed, so a real flight still passes. It is accuracy-blind (it looks at the jump, not the reported accuracy) and your own GPS reports always bypass it. Users can disable it if a legitimate fast movement is ever dropped. A newspeed_gate_enabledlabel/description is added to all ten locales.roundtrip_confirm_enabled("Recover legitimate return trips"). When a long jump A -> B is accepted, the pre-jump position A is anchored in memory (radiusROUND_TRIP_ANCHOR_RADIUS_M = 200 m, TTL ~15 min). A later crowd fix that the forward speed gate would otherwise reject as a teleport is accepted instead if it lands back near A within the TTL -- recovering a genuine return trip rather than stranding the tracker at the far position. It only ever applies where the speed gate would otherwise reject, so it can never loosen validation for normal fixes. A newroundtrip_confirm_enabledlabel/description is added to all ten locales.Fixed
Changed
const.py/manifest.json/pyproject.toml, an invariant anchored incustom_components/googlefindmy/AGENTS.md), and stamps the published artifact. This is where most of the non-test line count lives (release.yml, newrelease-stamp.yml, newscript/stamp_version.py,pyproject.tomlPSR config). Reviewer note on the version literal: the automation emits a four-part PEP 440 version (1.7.14.0); the on-disk files therefore read1.7.14.0rather than1.7.14. Branch and shipped HACS asset always carry the stamped value. The automation is branch-neutral by design so upstream can adopt it, but it is dev/CI only -- BSkando can keep or drop the two workflow files independently; nothing shipped to Home Assistant users depends on them.Removed
Compatibility
config_entryschema version is unchanged (CONFIG_ENTRY_VERSION = 2); existing installs are unaffected. Both new options default to on and preserve prior behaviour for normal fixes (the speed gate only rejects physically impossible jumps; the round-trip recovery only accepts fixes the gate would otherwise reject). The round-trip anchor is RAM-only (it does not survive a restart) -- intentional, cheap, and self-healing.Security/Privacy
Tests
test_cache_speed_gate.py(+447),test_cache_round_trip.py(+816),test_location_request_r6_name_sweep.py(+373),test_stamp_version.py(+220),test_hacs_validation.py(+56), plus small additions to coordinator/plus-code/hypothesis/surfacing tests.Risk & Rollback
custom_components/**/*.py= 7 files, +432 / -53), the two behavioural changes are default-on but conservative (reject only the physically impossible, recover only the otherwise-rejected), and there is no schema change or data migration. The main new state is the RAM-only round-trip anchor.