feat(#292): the atlas opens coloured by type, not by our own pipeline - #304
Merged
Conversation
`source` and `height` don't tell you much by colour. `source` describes OUR pipeline — lidar vs OSM tag vs estimate — which is honest and useful, but it is metadata about how we know a height, not about the place. `height` re-states what the 3D geometry already shows: tall buildings are visibly tall. Only `type` says where the city lives, works and makes things — green residential across the North Shore, cyan commercial downtown, orange industrial pockets. 72% of buildings come back `untyped in OSM (building=yes)`. That argues FOR this default, not against it, and buildings.ts already said so directly above ColorBy: "it shows exactly how much of the city is unmapped — which is the Build Plan's contribution loop ... The untyped bucket is the ask, rendered." The codebase knew; it just never made it the default. The target card already closes the loop with "Tag it and it appears here on the next load." Made the default ONE constant rather than flipping two literals. AtlasViewer held it in both a useState and a useRef — the ref lets the main effect read the live mode without depending on `colorBy` and tearing the viewer down on every toggle. Two literals disagree only on FIRST PAINT, which is the hardest place to notice. DEFAULT_COLOR_BY lives in buildings.ts next to the ColorBy type it instantiates, imported by both sites, so drift is now impossible rather than merely unlikely. Pinned by a test that was proven to bite: reverting the constant to 'provenance' fails it with "expected 'provenance' to be 'type'", and restoring passes. A product decision this visible should break a test, not slip through. Verified live: the `type` chip is active and the legend shows the six type buckets on first paint — chip and geometry agree, which is the invariant the single constant exists to protect. 18/18 buildings tests, type-check and lint clean.
1 task
This was referenced Jul 17, 2026
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #292.
The atlas now opens coloured by
type. Owner's call, and the right one.Why type
sourceandheightdon't tell you much by colour:sourcedescribes our pipeline — lidar vs OSM tag vs estimate. Honest and useful, but it's metadata about how we know a height, not about the place.heightre-states what the 3D geometry already shows you: tall buildings are visibly tall.typeis the only mode that says something about the city — green residential across the North Shore, cyan commercial clustered downtown, orange industrial pockets.The 72% argues FOR it, and the codebase already said so
5769 of 8031 buildings come back
untyped in OSM (building=yes). I initially raised that as an objection. It isn't — andbuildings.tswas already making the counter-argument directly aboveColorBy:The codebase knew. It just never made it the default. The target card already closes the loop: "Tag it and it appears here on the next load."
One constant, not two literals
The interesting part isn't the value — it's that
AtlasViewerheld the default twice:The ref exists so the main effect can read the live mode without taking
colorByas a dependency and tearing the viewer down on every chip click. That's sound — but it means two literals that must never disagree, and they'd disagree only on first paint: the chips would say one thing while the geometry rendered another. The hardest place to notice, and no test would catch it.So
DEFAULT_COLOR_BYnow lives inbuildings.ts, next to theColorBytype it instantiates, imported by both sites. Drift is impossible rather than merely unlikely. It also belongs there on the merits — the default colour mode is a property of the colour-by domain, not of the viewer component (andbuildings.tsis pure, so it's testable without mocking Cesium).The test was proven to bite
A test nobody has seen fail is a comment:
A product decision this visible should break a test, not slip through.
Verified
typechip active, legend shows the six type buckets, zero source categories — chip and geometry agree, which is the invariant the single constant exists to protect.buildings.test.ts(was 16), type-check and lint clean.Not in this PR
typechip was unreachable in the first place, and it's a layout bug, not a default.🤖 Generated with Claude Code