Repository navigation
Let a dimension hold no value instead of refusing it or filing it at 0 - #11
Merged
Merged
Conversation
0.9.1 stopped `null` and `[]` from filing a device under slot 0 by refusing them. That fixed #8's leak but left, as #10 reports, no way to say "this vehicle belongs to no client" on a `multi` dimension short of a sentinel value -- one that takes a slot of the capacity, shows up as a filterable value, and has to be agreed by every writer -- and a device reported without `dims` still landed in slot 0, matching the filter of whatever value was reported first. A device can now hold no value on a dimension: it has no cell in any filter shape that includes it. It still shows unfiltered and under filters on its other dimensions, and matches no filter on that one. The index already stores devices with no cells -- every device in a collection without dimensions is one -- so nothing is reserved and the snapshot format does not change. - `null` or `[]`, on any dimension: no value (was a 400 since 0.9.1) - absent, on a `capacity` dimension: no value (was slot 0) - absent, on a declared dimension: value 0, unchanged; that label is one the collection chose, and it is what a missing `category` has always meant Restore had to learn the difference. It filled any device without cells with the defaults, to keep a device findable when a config change left none of its cells valid; a device saved with no value would have been filed under the first label of a declared dimension on every restart. It now fills only a device that had cells and lost all of them. Devices already filed at slot 0 by an earlier version stay there until they are reported again with `dims`. A `null` inside a list is still a 422: `[1, null]` is a malformed value, not an empty one. The Node client declares `null` as a dimension value, which the server now accepts. Refs #10 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.
Points 2 and 3 of #10, as agreed in the discussion there. Point 1 (per-device rejection, opt-in) is a separate PR. Intended for
0.10.0, because devices reported withoutdimsbehave differently.Behaviour
A device can hold no value on a dimension: it has no cell in any filter combination that includes that dimension.
capacitydimensionvaluesdimensionnull/[]A sentinel such as
"none"is no longer needed, and no value takes no slot of thecapacity.Why no slot is reserved
The index already stores devices with no cells; every device in a collection without dimensions is one. So "no value" is just an empty cell list for that dimension's filter combinations. Nothing new is written to snapshots and the format doesn't change.
Changes
schema.rs(cells_for): an empty value list, or an absentcapacitydimension, now resolves to no value instead of slot 0 or a 400.collection.rs(restore): restore used to fill any device without cells with the defaults. That exists for a device that lost all its cells to a config change. A device saved with no value would have been filed under the first label of a declared dimension on every restart, so restore now fills only a device that had cells and lost them all.geojson.rs: comments only. A GeoJSONnullalready reached the schema as present-but-empty since 0.9.1.README.md, client README,index.d.ts: describe "no value", andnullis now part of the client'sdimsvalue type.Not changed
dims.[1, null]is still a 422: a malformed value, not an empty one.capacitydimension they now become "no value" instead of slot 0, but the previous value is still lost. Keeping the other dimensions' values deserves its own issue; per 0.9.1: um dispositivo com dims vazio recusa o lote inteiro, e multi não tem como expressar "sem valor" #10 it doesn't affect current users, who declare one dimension per collection.Testing
Four new tests in
tests/regressions.rsreplace the 0.9.1 test that expected the 400s:a_device_with_no_value_matches_no_filter_on_a_capacity_dimension: compactnulland[], absentdims, GeoJSONnull, and clearing a value with a patch. The filter for the first value matches only its real device, no slot is used, and[1, null]is still 422.an_empty_multi_value_is_no_client_and_does_not_sink_the_batch: 0.9.1: um dispositivo com dims vazio recusa o lote inteiro, e multi não tem como expressar "sem valor" #10's reproduction.f.clientId=7returns exactlyaande2.a_declared_dimension_keeps_value_zero_for_an_absent_value: an absent declared dimension still takes label 0, and[]there is no value.a_device_with_no_value_survives_a_snapshot_without_one: a snapshot round-trip with a declared and acapacitydimension.All four also call
verify()to check index invariants. Run against the code before this change, all four fail; the snapshot test also fails with only the restore change reverted.Run locally in
rust:1.98-slim-bookworm, with the same steps as CI:cargo fmt --checkandcargo clippy --all-targets --all-featureswith-D warnings: clean.cargo test --release: all pass.npm testagainst the new binary: 18 unit, 41 integration and 29 CLI tests pass.🤖 Generated with Claude Code