Skip to content

feat: store and read landmark content in ordinary maps - #1757

Merged
timohueser merged 53 commits into
developfrom
codex/ra09-landmark-map
Sep 15, 2026
Merged

timohueser merged 53 commits into
developfrom
codex/ra09-landmark-map

Conversation

@timohueser

@timohueser timohueser commented Sep 15, 2026

Copy link
Copy Markdown
Owner

Add optional landmark content to ordinary OBCM maps. OBCM v16 uses a 57-byte header and one bounded section with a latitude index, fixed source-linked records, UTF-8 text/credits, and independent 216 × 240 RGB222 photo streams. The decoder uses a 4 KiB history window and bounded reads. Pack, cut, bake and cache paths carry the content; streaming native/web assembly is reviewed separately in PR #1755 and is included in this integration branch.

This PR implements the data portion of #1744. The mutable framebuffer preparation phase is delegated in a separate worktree. Final real-data flat-card simulator acceptance remains open. No UI layout is changed here.

Validation so far: whole obc-formats, obc-reader, obc-pack, obc-bake and obc-vectors suites pass; the whole obc-web-assemble suite passes after normal cutter/native CLI regeneration of v16 fixtures, including text/credit/photo readback. The packer checks real photo digests before cache reuse. Independent adversarial source review is pending. The final shipped demo repack and shared v16 fixture packages are in progress, so this remains draft.

Public format documentation is updated in separate docs commits. No final local snapshot sweep or shipping resource build has been run; the integrated release step owns those budgets. Hardware acceptance remains pending.

Depends on PR #1753 and the RA08 country follow-up #1756. Do not close #1744 until the pixel phase and real production acceptance are complete.

CI dependency and browser size evidence

The final dependency fixes include the standalone board decoder lock entries and regenerated licence notices (7af2fde7). CI at 2c562608 measured the ordinary browser assembler at 689,089 bytes raw and 272,582 bytes compressed with its JavaScript. Only miniz_oxide and adler2 enter the production dependency graph; the map packer, GEOS, renderer and App remain absent. Required landmark QID/content/schedule merging and streaming increase the assembly engine. Under the existing structural-guard policy, 6b8bf966 sets the raw budget to 744 KiB (about 10 percent space above measurement); the 276 KiB compressed budget stays unchanged. No device or verification limit changes. Details: docs/assets/ride-assistant/implementation/ra09-ci-evidence.md.

Independent delta review accepted these build changes and the dependency/size evidence without repeating tests or builds. Whole-suite registry and documentation link checks passed. The latest develop weather removal is being integrated separately. Regional bake → cut → assemble and real flat-card simulator acceptance remain in progress; hardware acceptance is pending.

# Conflicts:
#	specs/OBCM_Spec.md
#	testing/suites.toml
@coderabbitai

coderabbitai Bot commented Sep 15, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@timohueser

Copy link
Copy Markdown
Owner Author

Independent adversarial review of data head 90f3bcaef694080af0f94bb3a64b3b771629f77f against codex/ra03-route-facts is complete. Two findings need fixes:

  • P2 — Keep landmark-only cells. In host/obc-pack/src/cut.rs:1019, CellArtifact.empty checks geometry, service POIs, navigation nodes and dropped records, but does not check the new landmark collection. host/obc-bake/src/planet.rs:1226 uses that flag to delete the emitted cell and record KnownEmpty. A valid information-only landmark in an otherwise empty cell is lost. Include landmarks in the empty decision and exercise a landmark-only cell in the existing cut or planet suite.
  • P2 — Load landmarks for the requested cut coverage. In host/obc-pack/src/cut.rs:299-303, the landmark loader uses source_extent.unwrap_or(compute_bbox(ing)) and ignores an explicit opts.bbox and selected cell coverage. Normal CellBakery sets the requested crop in opts.bbox while source_extent is None. Thus a landmark within the requested crop and selected cell, but outside the retained OSM feature extrema, is discarded before cell ownership is checked. Load against the requested coverage and let the existing half-open cell bounds select ownership. Cover a landmark outside the feature extrema but inside a selected cell.

When fixing these paths, change the cell cache identity so an existing v16 cell made with the old cut policy is rebuilt. The landmark fingerprint currently hashes the landmark loader, format module and Cargo.lock, but not these cut decisions.

Review covered the new format and lazy reader bounds, UTF-8 handling, query cancellation and paging, the bounded photo decoder and its miniz dependency checks, OSM joins and shared hours, serialization, cut/bake cache paths, vectors, and v16 contracts. No other blocking finding in that scope. The five updated web fixture cells match their registered hashes and v16 headers. No builds or test suites were repeated. The mutable photo phase, corrected real demo and shared v16 package publication remain declared acceptance work; this review does not mark them complete.

@timohueser

Copy link
Copy Markdown
Owner Author

Delta review of the cut fixes is complete: no blocking findings remain in this delta. The empty-cell decision includes landmarks; requested bbox or selected-cell bounds feed the content loader; the existing half-open ownership rule still assigns seam sites once. Bbox::microdegree_bounds uses widened integer ceiling/floor division, including negative coordinates, and both cut and whole-map paths reuse it. This resolves the exact east-edge float rounding case. Cell recipe version 3 invalidates prior cell-cache entries.

The new whole-path cut test checks a landmark outside OSM feature extrema with and without an explicit bbox, its east-cell ownership, and its nonempty artifact status. The existing bbox parser test now checks signed integer edges. Author reports all 14 cell-cut tests pass; broader affected checks are running. I inspected only the delta and did not rerun suites, builds, snapshots or resource checks. Approval remains subject to those checks and CI.

@timohueser

Copy link
Copy Markdown
Owner Author

Independent adversarial delta review of 53233340 and documentation 8049a6d2: one remaining finding.

[P2] Update the selected-region price expectation as well. apps/obc-desktop/e2e/launch.py:139 still waits for .parts .price to contain 994 B, while the preceding accessible-label assertion now correctly expects 1010 B. The fixture selects the two fine cells of 569 and 441 bytes, so the selected-region price is also 1010 B. Linux launch will pass the corrected Add-button wait and then time out at this next wait. Update the price expectation to match the same fixture total.

The separate v16 terrain-staging assertion and the inherited exact App allocation of 48,480 bytes are consistent with the source and retained CI evidence. Review used source and existing CI logs only; no suite, sweep, or resource image was repeated.

@timohueser

Copy link
Copy Markdown
Owner Author

Independent delta review of 5dc77d81 and documentation a70a19c5: the remaining finding is resolved. Both the Add-button label and selected-region price now require the fixture total of 1010 B. No new findings. This is source inspection only; Linux launch remains the CI gate and was not repeated locally.

@timohueser

Copy link
Copy Markdown
Owner Author

CI correction at b58c95f4, 14209178. Only the independently reviewed fixes from 0bbd3b2d and 43fb5328 were applied; an equivalent storage invariant was retained where it already existed. No child branch ancestry was merged.

The storage test checks that the fixture fits one sparse page, while keeping the actual page-allocation, pinned-revision and reuse checks. Six corridor route_bytes counters account for seven-byte OBCR point records instead of six: each changes by exactly 7/6. Row counts, read calls, map bytes and render hashes are unchanged.

The root orchestrator independently reviewed both deltas before propagation. Validation on the fix branches: whole host-core suites with exact catalog fixtures passed; whole release benchmark suite passed (10 tests); the normal release golden check matched all 17 records; registry checks passed. These identical deltas did not repeat tests, a snapshot sweep or a shipping image build. CI must pass on this PR head before merge. No public documentation changed.

@timohueser

timohueser commented Sep 15, 2026

Copy link
Copy Markdown
Owner Author

Snapshot input staging correction at 65a53988. The script imports only the valid plain and waypoint route vectors. Ride samples read their original binary directly, and all three cleanup traps remove the temporary route and ETA directories. Intentionally invalid format vectors remain unchanged.

The root orchestrator independently reviewed c3931395..51371533 and confirmed that all four propagated staging deltas have identical patch IDs. The whole firmware Python suite passed (86 tests). Bash syntax, diff and suite-registry checks passed. No repeated checks, snapshot sweep or shipping image build ran after review. CI remains the gate before merge.

Base automatically changed from codex/ra03-route-facts to develop September 15, 2026 10:45
@timohueser
timohueser marked this pull request as ready for review September 15, 2026 11:00
@timohueser
timohueser merged commit 32bdb00 into develop Sep 15, 2026
33 of 34 checks passed
@timohueser
timohueser deleted the codex/ra09-landmark-map branch September 15, 2026 11:00
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.

RA09 — Store and read compressed landmark content in ordinary maps

1 participant