Remove the null-MinimumZoom branch and ExplicitZoomLevel #209

Merged
rob merged 3 commits from retire-explicit-zoom into main 2026-08-21 12:34:03 +00:00
Owner

Closes task 266. With tasks 251 and 254 both landed, the only surviving producer of a MapZoomRequest (Home.HandleCurrentLocation) always names a floor, so MinimumZoom was never null and LeafletMap.ExplicitZoomLevel was never reached. Confirmed by search, not assumption — no other constructor call exists anywhere in the tree.

MapZoomRequest.MinimumZoom is now a required int. ExplicitZoomLevel and the setView branch it fed are removed. Tests that existed only to cover that branch are deleted, not inverted; the tests covering the surviving RequestId mechanism are kept and updated to the new required parameter.

ADR-0172 records the removal and partly supersedes ADR-0132 and ADR-0166, which are the reason the branch was still there. Both, plus ADR-0167, carry Corrected fields and inline blockquotes at the claims this change falsifies, rather than being rewritten.

Closes task 266. With tasks 251 and 254 both landed, the only surviving producer of a `MapZoomRequest` (`Home.HandleCurrentLocation`) always names a floor, so `MinimumZoom` was never `null` and `LeafletMap.ExplicitZoomLevel` was never reached. Confirmed by search, not assumption — no other constructor call exists anywhere in the tree. `MapZoomRequest.MinimumZoom` is now a required `int`. `ExplicitZoomLevel` and the `setView` branch it fed are removed. Tests that existed only to cover that branch are deleted, not inverted; the tests covering the surviving `RequestId` mechanism are kept and updated to the new required parameter. ADR-0172 records the removal and partly supersedes ADR-0132 and ADR-0166, which are the reason the branch was still there. Both, plus ADR-0167, carry `Corrected` fields and inline blockquotes at the claims this change falsifies, rather than being rewritten.
Regenerate the ADR index table after rebasing onto main
Some checks failed
CI / build (pull_request) Successful in 5m47s
CI / container-images (pull_request) Failing after 4m53s
CI / e2e (pull_request) Has been cancelled
6bb0826025
rob force-pushed retire-explicit-zoom from 6bb0826025
Some checks failed
CI / build (pull_request) Successful in 5m47s
CI / container-images (pull_request) Failing after 4m53s
CI / e2e (pull_request) Has been cancelled
to 77a656d3dd
All checks were successful
CI / build (pull_request) Successful in 3m49s
CI / container-images (pull_request) Successful in 2m45s
CI / e2e (pull_request) Successful in 4m5s
2026-08-21 12:25:23 +00:00
Compare
rob left a comment

Verdict: mergeable

Verified independently rather than taking the PR body's claims on trust:

  • Searched the whole tree for new MapZoomRequest(...): Home.razor.cs:313 is the only constructor call anywhere in src/ or tests/, and it always supplies CurrentLocationMinimumZoom. No JS interop or external parameter set reaches ZoomRequest another way. ExplicitZoomLevel no longer exists as an identifier anywhere — the two remaining hits are prose (<c>ExplicitZoomLevel</c>) in doc comments. The dead-code claim holds.
  • The two deleted tests only ever covered the removed setView/null branch; nothing live lost coverage. The five kept/renamed tests still assert real behaviour (RequestId reconciliation, no redundant calls, interop-failure handling, never-zoom-out).
  • dotnet build -c Release is clean (0 warnings/errors) and dotnet test on PlaceMark.Architecture.Tests (23/23, including AdrFiles_Corrected_HasAMatchingInlineMarkerInBothDirections and the reciprocal supersession check) and PlaceMark.WebUI.Tests (914 passed, 4 pre-existing unrelated skips) both pass, all under the pinned 10.0.100 SDK.
  • Reciprocal ADR pointers check out by hand too: 0172's Partially supersedes field lists both 0132 and 0166; 0132's Partially superseded by field's continuation prose names 0172 (picked up by the architecture test's field-continuation parser, confirmed by reading its logic); 0166 gets its own dedicated Partially superseded by: [ADR-0172] field. Corrections are honest — original claims (16 as the chosen level, the "kept rather than deleted" judgement) stay legible, with dated inline blockquotes at the falsified sentences rather than rewrites.
  • LeafletMap.OnParametersSetAsync's ZoomRequest branch is gated behind _map is not null, which only becomes true after OnAfterRenderAsync(firstRender: true) runs — the same method #210's readiness wait hooks into via createMap. The two changes don't interact: this PR touches neither OnAfterRenderAsync nor createMap.
  • ADR-0172 doesn't collide with 0170/0171/0173 (0171 correctly absent, per your note it's reserved for task 271). No AI attribution in the three commits on this branch. No British English regressions in the touched files (the only American spellings found — center, _animateZoom — are pre-existing vendored Leaflet internals untouched by this diff).

CI: build job green as of this review; e2e and container-images were still running (SHA-verified via get_workflow_run on run 885, cross-checked against the run's own commit SHA — the run-793-by-URL-id lookup returns a stale unrelated run, exactly the trap called out, so don't trust that id directly). Worth a final glance at the e2e result before merging, per the note that main's suite is now healthy and a failure here would be meaningful.

Verdict: mergeable Verified independently rather than taking the PR body's claims on trust: - Searched the whole tree for `new MapZoomRequest(...)`: `Home.razor.cs:313` is the only constructor call anywhere in `src/` or `tests/`, and it always supplies `CurrentLocationMinimumZoom`. No JS interop or external parameter set reaches `ZoomRequest` another way. `ExplicitZoomLevel` no longer exists as an identifier anywhere — the two remaining hits are prose (`<c>ExplicitZoomLevel</c>`) in doc comments. The dead-code claim holds. - The two deleted tests only ever covered the removed `setView`/null branch; nothing live lost coverage. The five kept/renamed tests still assert real behaviour (RequestId reconciliation, no redundant calls, interop-failure handling, never-zoom-out). - `dotnet build -c Release` is clean (0 warnings/errors) and `dotnet test` on `PlaceMark.Architecture.Tests` (23/23, including `AdrFiles_Corrected_HasAMatchingInlineMarkerInBothDirections` and the reciprocal supersession check) and `PlaceMark.WebUI.Tests` (914 passed, 4 pre-existing unrelated skips) both pass, all under the pinned 10.0.100 SDK. - Reciprocal ADR pointers check out by hand too: 0172's `Partially supersedes` field lists both 0132 and 0166; 0132's `Partially superseded by` field's continuation prose names 0172 (picked up by the architecture test's field-continuation parser, confirmed by reading its logic); 0166 gets its own dedicated `Partially superseded by: [ADR-0172]` field. Corrections are honest — original claims (16 as the chosen level, the "kept rather than deleted" judgement) stay legible, with dated inline blockquotes at the falsified sentences rather than rewrites. - `LeafletMap.OnParametersSetAsync`'s `ZoomRequest` branch is gated behind `_map is not null`, which only becomes true after `OnAfterRenderAsync(firstRender: true)` runs — the same method #210's readiness wait hooks into via `createMap`. The two changes don't interact: this PR touches neither `OnAfterRenderAsync` nor `createMap`. - ADR-0172 doesn't collide with 0170/0171/0173 (0171 correctly absent, per your note it's reserved for task 271). No AI attribution in the three commits on this branch. No British English regressions in the touched files (the only American spellings found — `center`, `_animateZoom` — are pre-existing vendored Leaflet internals untouched by this diff). CI: build job green as of this review; e2e and container-images were still running (SHA-verified via `get_workflow_run` on run 885, cross-checked against the run's own commit SHA — the run-793-by-URL-id lookup returns a stale unrelated run, exactly the trap called out, so don't trust that id directly). Worth a final glance at the e2e result before merging, per the note that main's suite is now healthy and a failure here would be meaningful.
rob merged commit d8c172aa25 into main 2026-08-21 12:34:03 +00:00
rob deleted branch retire-explicit-zoom 2026-08-21 12:34:04 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/PlaceMark!209
No description provided.