Merging kinks: Another serialization rabbit-hole

[pixelcmtd]

While trying to merge two kinks for KCC v0.9, I noticed that the current serialization format doesn't allow us to carry over ratings in that case. Merging two kinks ["A", [""]] and ["B", [""]] into a new kink ["C", ["a", "b"]] can only reasonably get a new ID, which throws away existing ratings. But this is a case where the serialization format could help, and therefore imo it should. This might, however, be quite difficult to implement.

Alternative 1: Flat format (checkData = { ratings: optional<validRating>[] })

We could overhaul the entire serialization format to just store a list of ratings:

type kink = [string, [""], [number]]
          | [string, [""], [number], string]
          | [string, [string, string], [number, number]]
          | [string, [string, string], [number, number], string]

type checkData = { ratings: (validRating | undefined | null)[] }

// decodeKinkCheck:
kid.map(i => s.ratings[i]) // .filter(r => r || r === 0)

Alternative 2: Special cases (kink.id = number | [number, number])

Alternatively, we can keep most of the serialization as-is, and patch up that specific case:

type kink = [string, positions, number | number[]]
          | [string, positions, number | number[], string]

type checkData = { ratings: (validRating[] | undefined | null)[] }

// decodeKinkCheck:
typeof kid === "number" ? s.ratings[kid] : kid.flatMap(i => s.ratings[i])

[Drezil]

I would vote for Alternative 2, because having such instances where 2 kinks get merged should NOT be some default thing that will occur nilly-willy often. It should be a special case (maybe in a "convert legacy"-file) where all these historic conversion-thingies are done as a long list of "one offs".

Also: do you want to support i.e. 0.7 -> 0.9 directly? Or is the path always gonna be 0.7 -> 0.8 -> 0.9? One could generate a "list of one-off conversions" like migrate[from][to] = [migrations] with only "from +1 == to" being guaranteed and migrations[from].indices being the allowed implemented migrations..

[pixelcmtd]

Also: do you want to support i.e. 0.7 -> 0.9 directly? Or is the path always gonna be 0.7 -> 0.8 -> 0.9?

The goal of the serialization format has already been established as being as revision-independent as possible (reasoning in #29 and below). Therefore, we support "everything" (0.8 → 0.9, 0.9 → 0.6, ...), and this issue is pretty much the last edge case where the current format doesn't work perfectly.

It should be a special case (maybe in a "convert legacy"-file) where all these historic conversion-thingies are done as a long list of "one offs".

No, converting between revisions is not a one-off special case. Even if you were to disagree with the choices made in #29, you'll probably still agree that somehow "carrying over" your previous Draft¹ to a new KCC revision is a much better UX than having to fill it from scratch again. Therefore, we'll need to make something like 0.x → 0.9 (x < 9) possible in the client, without the server knowing x. So the client needs to have enough information to migrate any previous revision to, for example, 0.9.

¹ Draft is the word I've started using for a Check saved in localStorage

One could generate a "list of one-off conversions" like migrate[from][to] = [migrations] with only "from +1 == to" being guaranteed and migrations[from].indices being the allowed implemented migrations.

The migrations idea was ruled out a lot earlier. If I may quote myself (#21):

The only other option that might be worth exploring are explicitly defined migrations between Template revisions: "this kink moved somewhere else", "that one got removed", etc. The problem with that is the huge complexity of such a system. If we can in the end just say "kink [0, 1] got removed" ([0, 1] would be an example of a [category, kink] index) that's great, but it's quite a bit of very error-prone code to implement that. It's definitely doable, I can imagine a rough format in my head, but what would the cost-benefit look like?

It would also increase our bundle size quite a bit, not just because we'd need a somewhat complicated algorithm to apply migrations, but also because we'd need to pass migrations to the KinkCheck component. I don't think this would undo the 10KB reduction from ea0fa39, especially since we could hopefully get rid of kink IDs, but it wouldn't be free either.

My biggest concern, however, remains complexity. Not just complexity of the algorithm that applies migrations, but also the DX of adding new revisions. Currently the story is as simple as cp templates/kcc/0.8.yaml templates/kcc/0.9.yaml and editing it. If you add a kink, you look for the current max ID and add 1. Moving kinks around is a simple cut-and-paste. What would the story look like with migrations? Do I need to add a field to the revision that specifies the migrations? Does that go in a global file, like templates/kcc/migrations.yaml? And how easy can we really make writing migrations before the function that applies them becomes too complicated? What's the story for testing? Currently we only test that a revision doesn't internally reuse IDs, but a script that spits out a list of possible conflicts is very easy to write. For migrations I can definitely see it being doable as well, but again: At what cost?

I would vote for Alternative 2, because having such instances where 2 kinks get merged should NOT be some default thing that will occur nilly-willy often.

Non sequitur: I really don't see why this decision would be made based on how often a special case occurs. Either we need to have code that deals with the special cases or we change the format so that those special cases don't matter. The frequency of special cases doesn't change any of that.

I'm personally leaning a bit towards Alternative 1, because it probably even reduces the total LoC, while decreasing the size of the encoded checkData JSON. But it would require every kink to have an array of indices, which might make Template editing slightly more tedious. Alternative 2 on the other hand would set the slightly weird serialization format in stone, and require extra code to deal with weird edge cases like merging kinks, but doesn't require any changes to the existing Template revisions (KCC 0.8 and 0.9 [in-progress], unhealthy-stimming 0.1, and april-fools 0.1).

[Drezil]

No, converting between revisions is not a one-off special case. Even if you were to disagree with the choices made in #29, you'll probably still agree that somehow "carrying over" your previous Draft¹ to a new KCC revision is a much better UX than having to fill it from scratch again. Therefore, we'll need to make something like 0.x → 0.9 (x < 9) possible in the client, without the server knowing x. So the client needs to have enough information to migrate any previous revision to, for example, 0.9.

thats exactly like i meant. Like a chain of "you get from 0.x to 0.(x+1) with this changes: ..; you get from 0.(x+1) to 0.(x+2) with these changes; ...; you get from 0.(x+n) to 0.9 with these changes". Those should be generatable from the yml at compile-time and just a static js-asset that gets loaded into the migration thingy.

My biggest concern, however, remains complexity. Not just complexity of the algorithm that applies migrations, but also the DX of adding new revisions. Currently the story is as simple as cp templates/kcc/0.8.yaml templates/kcc/0.9.yaml and editing it. If you add a kink, you look for the current max ID and add 1. Moving kinks around is a simple cut-and-paste. What would the story look like with migrations? Do I need to add a field to the revision that specifies the migrations? Does that go in a global file, like templates/kcc/migrations.yaml? And how easy can we really make writing migrations before the function that applies them becomes too complicated? What's the story for testing? Currently we only test that a revision doesn't internally reuse IDs, but a script that spits out a list of possible conflicts is very easy to write. For migrations I can definitely see it being doable as well, but again: At what cost?

Cost: One-Time definition of legal state-changes from AnyVer -> AnyVer as EDSL. diff v1.yml v2.yml | some_script should generate those changes, first into the internal EDSL of changes and then into a static .js that is basically an array of functions that apply to one Draft-object of version X until it is at the desired version.

So from a dev perspective this stays as "copy the yml and adapt". the compile then generates the js-playload that is only loaded if a version-mismatch is found to migrate things over.

The serialization-format is still arbitrary between version because of the dynamically generated functions inside the .js. And the migration.js (or however called) is also just a static asset that gets only appended to when a new version gets added (so it does not need to i.e. recreate 0.1 -> 0.2 if we are on 0.9 and so much changed that drafts won't even parse the old format correctly).

Worst case: one could write such a migration.js by hand before generating them in a script.

Bot those are just my 2 cents - feel free to ignore :)