Rejustifies the duration subset rule - #67
Merged
Merged
Conversation
Predicator 9.0 settled the eight-key duration contract (px-69c), so Shape.duration?/1's @doc no longer explains its any-non-empty-subset rule by citing a seven-key parser. The rule is unchanged; the reason is now the viewer's own: statifier-ui renders a value stream it did not produce, and a duration short a unit should still read as a duration. The pinned "durations from real predicator expressions" test was passing incidentally - under 9.0 every expression yields all eight keys, so tightening the rule to require exactly eight left it green. It now asserts the eight-key contract explicitly and asserts that dropping a unit from real evaluator output still infers as a duration, which is the tolerance the rule exists for. Value.encode/1 is unchanged: filling :milliseconds with 0 writes a unit the contract declares, not one it lacks. Refs: sui-cw0
This was referenced Sep 2, 2026
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.
Why
sui-cw0(mirrorspx-69c) was filed while predicator's evaluated durationshad an expression-dependent key set: seven units normally, eight when the
expression mentioned
ms, against a declared type of eight.px-69cclosed onthe eight-key branch (predicator PR #176, shipped in predicator 9.0.0), so the
justification this repo wrote against the seven-key output is now false, even
though the code it justifies is still right.
What
StatifierUI.Shape.duration?/1still accepts any non-empty atom-keyed subsetof the eight units. The
@docno longer explains that by citing a parser bug;it explains it as the viewer's own tolerance - statifier-ui renders a value
stream it did not produce (a hand-written fixture, a decoded wire message, a
datamodel from an older engine or another interpreter), and a duration short a
unit should read as a duration rather than collapse into a seven-field map.
The pinned test
"durations from real predicator expressions"was passingincidentally and now pins the rule explicitly. See the sabotage evidence below.
Contract re-read, with citations
Resolved dependency:
mix.lock:27pinspredicatorat9.0.0(>= 9.0.0, asrequired).
deps/predicator/lib/predicator/types.ex:37-46-@type durationdeclaresall eight units,
milliseconds: non_neg_integer()among them, and themoduledoc at
types.ex:24-29states "Every unit the expression did not nameis present and
0, so the map is always this wide".deps/predicator/lib/predicator/duration.ex:36-48-new/1builds all eightvia
Keyword.get(opts, :milliseconds, 0).Live probe against the resolved dependency, all eight keys every time:
Sabotage evidence
Temporarily tightening
duration?/1fromMapSet.subset?/2toMapSet.equal?/2(require exactly eight keys), then reverting:seven-key test, the partial-duration test.
"durations from real predicator expressions"stayed green. It was passing incidentally, which is theoutcome this bead was filed to prevent.
"durations from real predicator expressions", failing onShape.infer(Map.delete(value, :milliseconds)) == :duration.The rule is reverted to
MapSet.subset?/2; the sabotage was an experiment, nota change. The test now asserts both halves: that real evaluator output carries
all eight keys (so a regression upstream fails at the dependency), and that
dropping a unit from that real output still infers as a duration (the tolerance
the subset rule exists for).
Value.encode/1is correct as it stands - unchanged herelib/statifier_ui/value.ex:191-196maps over all eight@duration_unitswithMap.get(duration, unit, 0), soencode/1fills an absent:millisecondswith
0. Under the eight-key contract that writes a unit the value domaindeclares (
types.ex:45) and thatDuration.new/1itself produces - it iscanonicalization, not invention. The bead's encode-side worry applied only to
the seven-key branch, which predicator did not take. No change was needed and
none was made; the file is untouched on this branch.
Residual, deliberately not fixed here:
value.ex:24-29's moduledoc carriesthe same stale parenthetical ("predicator's parser emits seven, omitting
:milliseconds"). It is prose only and outside this branch's declared fileset, so it is reported rather than edited. Worth a follow-up bead.
Sibling bead
st-4epqin statifier-ex (alsomirrors: px-69c) is CLOSED - "Acceptancecriteria satisfied via st-793d (predicator 9.0 consumption bump, merged PR
#175); no engine-side work remained". Its notes record a seven-versus-eight
audit of that repo finding no key-set assumptions. Nothing is owed there.
Notes
mix qualitygreen on the rebased HEAD (attested, not scoped): format,compile, dependencies, doctor, credo, 726 tests / 92.9% coverage, dialyzer.
Gettext and Sobelow are the two permanent not-applicable skips.
behavior change, per
changelog.d/README.md.mainat4ad5a9d; no commits replayed, no conflicts.px-69c- not closed here. NoClosesline by design:this half stays open for the operator.
Refs: sui-cw0