WIP support sets (well, kinda of) by adinapoli · Pull Request #79 · iconnect/api-tools

adinapoli · 2023-03-09T13:55:24Z

Support `Set T` in schemas

Rewritten from scratch on top of master, addressing @adamgundry's review of #79.
Schemas can now say Set T; generated Haskell uses Data.Set.Set; JSON and CBOR
stay plain arrays.

Review points from #79

JSON is just the element list. Dropped the CBOR-tagged-object shape and
jsonParseCborSet. FromJSONWithErrs (Set a) decodes a JS.Array via
Set.fromList (order-insensitive, dedups); encoding is aeson's
ToJSON (Set a), i.e. ascending.
Set T syntax, not {T}. Note this makes Set a reserved word — a type
previously called Set must now be written 'Set'.
No DefValSet; DefValList covers both, and defaultValueForType returns
it for TySet too.
No fromDistinctAscList anywhere. Nothing assumes sorted or distinct input.
No special CBOR set support. Data/Binary/Serialise/CBOR/Extra.hs is
untouched, so the licensing question goes away with it.
Generic/specific CBOR agreement. This is the bit worth a look. Value gains
a SetList constructor which preserves wire order — the decoder never sorts,
so an Ord instance on the concrete type that disagrees with the ordering of
the corresponding Values can't cause a mismatch. SetList also encodes with
encodeListLen (definite length), matching serialise's encodeSetSkel, where
lists use the indefinite-length encoding. prop_cborGeneric compares FlatTerms
exactly, so both of these are load-bearing.

SetList is deliberately a separate constructor from List: Value.encode has no
schema to hand and dispatches purely on the constructor, so it can't otherwise tell
which length encoding to use.

Also in here

Generated traversals descend into set-valued fields. Set isn't Traversable,
but traversing via the element list works; previously traverser' returned
Nothing for TySet and the generated traversal silently left sets alone.
Note this can shrink a set if the function is not injective.
Set added to Data.API.Scan.keywords.
Test coverage: Set fields in both example schemas, plus Foo.ens :: Set AnEnum
in the migration schemas so the 2.1 enum rename migrates through a set
(UpdateSet, expectSetList), and field added noset :: Set string at 2.6 for
the implicit set default. Decoding cases for out-of-order/duplicate arrays and
for objects being rejected.

Known limitation

Set element types need Ord, and records/unions don't derive it by default, so
Set SomeRecord needs datatypesTool' to add it. Documented in the tutorial and
changelog. Inferring it from the API (deriving Ord for types used as set
elements) would be nicer but changes the defaultDerivedClasses :: APINode -> [Name]
signature.

adamgundry

Thanks @adinapoli, I think sets would be very useful to support more natively in api-tools. I have a few concerns about the CBOR-related details and I think we can do something simpler, but the general idea seems good to me.

Add TySet with Set T syntax. Generated Haskell types use Data.Set.Set, while JSON and CBOR wire formats remain plain arrays: decoding accepts arrays in any order and drops duplicates, encoding uses the element order of the corresponding Set. This avoids bespoke CBOR set tags and reuses the existing list default value for set-valued fields.

Follow-up to "Support set types in API schemas", addressing gaps found while reviewing that change against Adam's review of #79. * Generated traversals now descend into set-valued fields. Previously traverser' returned Nothing for TySet, so traverser fell back to `const pure` and generated traversals silently skipped sets: values of the traversed type nested inside a `Set T` field were never visited. Data.Set.Set is not Traversable (its element type is constrained by Ord), but we can traverse via the element list, so add traverseSet and use it. Note this may shrink the set if the function maps distinct elements to the same value. * Add "Set" to Data.API.Scan.keywords, as the N.B. in Scan.x requires for any new keyword. * Cover TySet in the migration tests. UpdateSet, expectSetList and the implicit set default were previously untested: - Foo gains `ens :: Set AnEnum` in the start and end schemas, so the 2.1 AnEnum alternative rename migrates through a set, in both the JSON and the generic Value paths. - version 2.6 gains `field added noset :: Set string`, exercising defaultValueForType at TySet. * Add a test that the generated traversal visits set elements. * Document that set element types need Ord, and that records and unions do not derive it by default, so a set of such a type requires datatypesTool' (or the defaultDerivedClasses setting). * Note in the changelog that Set is now a reserved word, so a type previously called Set must be written 'Set'. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Adam's review of #79 asked for the JSON representation of a set to be simply the list of its elements, order-insensitive when decoding. That is what the FromJSONWithErrs instance now does, but nothing asserted it: add decoding cases covering an out-of-order array with a duplicate element, and an object being rejected (the shape the original branch accepted via jsonParseCborSet). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

adamgundry

This is looking pretty good, thanks @adinapoli!

adamgundry · 2026-07-29T10:32:22Z

+           -- | A set on the wire.  We retain the serialised order rather
+           -- than sorting in the generic representation, so that generic
+           -- CBOR decoding agrees with type-specific decoders.
+           | SetList ![Value]


Do we need to add SetList to the generic Value representation, or can we get away with List? In particular it would be nice if changing a type between a list and a set didn't need a data migration.

Ah hold on, I see this is addressed in the PR comments (but not in the code, that I could see):

Value gains a SetList constructor which preserves wire order — the decoder never sorts, so an Ord instance on the concrete type that disagrees with the ordering of the corresponding Values can't cause a mismatch. SetList also encodes with encodeListLen (definite length), matching serialise's encodeSetSkel, where lists use the indefinite-length encoding.

SetList is deliberately a separate constructor from List: Value.encode has no
schema to hand and dispatches purely on the constructor, so it can't otherwise tell
which length encoding to use.

So the problem here is that serialise encodes lists and sets differently on the wire, so it is nontrivial for us to avoid distinguishing them. Perhaps we will have to keep SetList after all, and accept that migrating between lists/sets changes the CBOR representation.

adamgundry · 2026-07-29T10:36:26Z

 module Data.API.Tools.Traversal
    ( traversalTool
    , traversalsTool
+    , traverseSet


I don't think this is needed in the public API?

Suggested change

, traverseSet

adamgundry · 2026-07-29T10:38:11Z

+-- | Traverse the elements of a 'Set.Set'.  This is not a lawful 'Traversal'
+-- (it cannot be, because 'Set.Set' is not a 'Functor'), but it is what the
+-- traversals generated by 'traversalsTool' use for set-valued fields.


Let's add this to the comments on traversalTool explicitly saying that if the schema uses sets, the generated traversal will be non-lawful as it may coalesce multiple elements that are mapped to the same value.

adamgundry · 2026-07-29T10:39:03Z

 -- | Represents the positions in a type to apply an update
 data UpdateTypePos
    = UpdateList UpdateTypePos
+    | UpdateSet UpdateTypePos


Hopefully if we can get rid of SetList then UpdateSet also becomes unnecessary?

adamgundry · 2026-07-29T10:44:19Z

+    = record
+        ints  :: Set integer // a set of integers
+        flags :: Set Flag    // a set of boolean newtypes
+        utcs  :: ? Set utc   // an optional set of UTC timestamps


As we discussed, it would be nice to be able to write Maybe (Set UTCTime) for ? Set utc and similarly Set Int instead of Set integer (while keeping backwards compatibility for the old syntax). I suggest we save that for a follow-up PR though.

adamgundry reviewed Mar 9, 2023

View reviewed changes

adinapoli force-pushed the adinapoli/experiment-in-supporting-sets branch from 8b97056 to 37cab74 Compare July 29, 2026 08:26

adinapoli and others added 2 commits July 29, 2026 10:53

adamgundry reviewed Jul 29, 2026

View reviewed changes

Uh oh!

Conversation

adinapoli commented Mar 9, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Support Set T in schemas

Review points from #79

Also in here

Known limitation

Uh oh!

adamgundry left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adamgundry left a comment

Choose a reason for hiding this comment

Uh oh!

adamgundry Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

adamgundry Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

adamgundry Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

adamgundry Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

adamgundry Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

adamgundry Jul 29, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

adinapoli commented Mar 9, 2023 •

edited

Loading

Support `Set T` in schemas