Abstract record builder

[joaosreis]

No description provided.

[Copilot]

Pull Request Overview

This PR abstracts the record builder implementation by introducing a generic BuilderItem typeclass and a new MemPackHeaderOffset typeclass. The changes enable the builder state machine to work with different item types while maintaining a consistent interface.

Key changes:

• Introduced MemPackHeaderOffset typeclass to make header offset configurable per type
• Created generic BuilderItem typeclass with associated Parameters type family
• Refactored ChunksBuilder.InMemory to use the new generic Builder.InMemory implementation

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
scls-format/src/Cardano/SCLS/Internal/Serializer/MemPack.hs	Added MemPackHeaderOffset typeclass and implemented it for RawBytes
scls-format/src/Cardano/SCLS/Internal/Serializer/Builder/InMemory.hs	New generic builder implementation with BuilderItem typeclass
scls-format/src/Cardano/SCLS/Internal/Serializer/ChunksBuilder/InMemory.hs	Refactored to use generic builder, implementing BuilderItem for ChunkItem
scls-format/src/Cardano/SCLS/Internal/Entry.hs	Implemented MemPackHeaderOffset for ChunkEntry
scls-format/src/Cardano/SCLS/Internal/Serializer/Reference/Impl.hs	Added MemPackHeaderOffset constraint to serialize function
scls-format/src/Cardano/SCLS/Internal/Serializer/Reference/Dump.hs	Added MemPackHeaderOffset constraint to affected functions
scls-format/src/Cardano/SCLS/Internal/Serializer/External/Impl.hs	Added MemPackHeaderOffset constraint and updated imports
scls-format/test/ChunksBuilderSpec.hs	Refactored tests to use helper function mkMachine'
scls-format/scls-format.cabal	Added new module to exposed modules list

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[qnikst]

In general I like the PR.

But to be honest it took some time for me to understand the abstraction behind and how we are going to use it. So let me phrase it here, to be sure we are on the same page:

We introduce an abstraction behind the chunk builder machine so we can use different encodings

We are going to use it in order to be able to keep different chunk encoding for example raw or zstd.

If so I'm ok, however I see that it seems it misses one scenario:

1. using zstd-e — because we abstract only final representation we no longer have an access to the entry data here, so can't encode entry by entry.

[joaosreis]

@qnikst

We introduce an abstraction behind the chunk builder machine so we can use different encodings

The idea is to be quite more general. Specifically, I would like to use this logic to also build metadata records, from metadata entries. I wanted to avoid duplicating much of the code for this purpose, and that's the motive for this PR. In other words, I would phrase it like:

We introduce an abstraction behind the chunk builder machine so that it can be used to build a record (chunk, metadata, ...) from a stream of abstract entries and an instance of a typeclass that defines how to build that record from those entries

---

Regarding

using zstd-e — because we abstract only final representation we no longer have an access to the entry data here, so can't encode entry by entry.

Hmm true, in this current implementation we lose access to custom entry encoding. We could require an entry encoding function on the BuilderItem type class for this purpose.

---

Regardless, I'm open to discuss this further because I feel this might be a case of "Don't Repeat Yourself" vs. "So much complexity in software comes from trying to make one thing do two things."
Personally, I always feel compelled towards the first one, but that might be an irrational decision sometimes :)

[qnikst]

I totally agree with the current approach and extending is as complex as adding one extra method to the type class, so I'm fine with the approach