feat(mapper): add wire-size-based chunking and ResultToGetTargetGraphResponse#206
Open
yushan8 wants to merge 3 commits into
Open
feat(mapper): add wire-size-based chunking and ResultToGetTargetGraphResponse#206yushan8 wants to merge 3 commits into
yushan8 wants to merge 3 commits into
Conversation
…Response Introduce internal/mapper/chunk.go with BySize (generic proto chunker bounded by real serialized size) and ChunkMetadata (map splitter). Add ResultToGetTargetGraphResponse to internal/mapper/target_graph.go, converting a targethasher.Result into chunked stream responses. Both old (core/common) and new (internal/mapper) implementations coexist — callsite migration follows in a subsequent PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This was referenced Jul 15, 2026
| // ResultToGetTargetGraphResponse converts a targethasher.Result into chunked | ||
| // GetTargetGraphResponse messages ready for streaming or storage. Each message | ||
| // is bounded by maxMessageBytes of real wire size (see chunkTargets). | ||
| func ResultToGetTargetGraphResponse(ctx context.Context, result targethasher.Result, maxMessageBytes int) ([]*tangopb.GetTargetGraphResponse, error) { |
Contributor
There was a problem hiding this comment.
I feel like this function is doing a lot of logic that maybe shouldn't be in mapper. This could be broken into two. First is taking targethasher.Result and processing it into optimized targets. This part can stay in orchestrator (optimized target needs an entity). Second is turning optimized targets into the proto response, so the mapper can do just this part.
…d proto steps Add entity.OptimizedTarget and entity.TargetGraph as proto-free domain types for target graphs. Split the monolithic ResultToGetTargetGraphResponse into: - ResultToTargetGraph: targethasher.Result → entity.TargetGraph (proto-free) - TargetGraphToProto: entity.TargetGraph → chunked proto responses This lets the orchestrator work with entity types while keeping proto conversion confined to the mapper and controller layers. Co-Authored-By: Claude Sonnet 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
2 participants
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.
Summary
Introduces wire-size-based chunking and a proto-free domain type for target graphs, separating entity conversion from proto serialization.
entity.OptimizedTargetandentity.TargetGraphas proto-free domain types for target graph data.ResultToTargetGraph: convertstargethasher.Result→entity.TargetGraph(proto-free, suitable for orchestrator use).TargetGraphToProto: convertsentity.TargetGraph→ chunked[]*tangopb.GetTargetGraphResponse(proto serialization + ID mapping + chunking).internal/mapper/chunk.gowithBySize(generic proto chunker bounded by real serialized size viaSize()) andChunkMetadata(map splitter by measured entry wire size).BySizealways returns at least one chunk (empty input → single empty chunk); non-first empty chunks return an error.core/common) and new (internal/mapper) implementations coexist — callsite migration follows in config: bound stream chunks by real message size, not estimated #203.This split enables a future PR to have the orchestrator work with
entity.TargetGraphdirectly, removing proto from the orchestrator layer entirely.Test plan
make buildmake test—ResultToTargetGraph(entity conversion),TargetGraphToProto(empty graph),ResultToGetTargetGraphResponse(chunking at various sizes),BySize/ChunkMetadataunit testsmake gazelle && gofmt -w . && goimports -w .Stack