Refactor controller: dedup shared pipeline and split RPCs into focused helpers#111
Closed
sbalabanov wants to merge 1 commit into
Closed
Refactor controller: dedup shared pipeline and split RPCs into focused helpers#111sbalabanov wants to merge 1 commit into
sbalabanov wants to merge 1 commit into
Conversation
…d helpers The four controller RPCs duplicated large amounts of pipeline scaffolding — revision validation, treehash lookup, cached-response drains, concurrent two-graph fetch, name/ID transposition, and INDIRECT->DIRECT promotion all lived twice in side-by-side ~700-line files. Pull the shared logic into small, well-named helpers and re-express each RPC as a top-to-bottom pipeline. - rpc_helpers.go: recordRPCResult defer used by every RPC; validateRevisionPair shared by GetChangedTargets and GetChangedTargetsAndEdges validators. - cache.go: cacheReader[T] generic interface plus readTreehash/ readTreehashPair/loadCachedResponses unify the cache scaffolding. - graphfetch.go: fetchTwoGraphs concurrently fetches both revisions with sibling cancellation on the first failure. - mappers.go: idMappers bundles the five canonical name->ID mappers and exposes transpose/chunkMetadata. - classify.go: promoteToDirectIfNeeded centralizes the INDIRECT->DIRECT promotion rules. Each RPC body shrinks dramatically — getchangedtargets.go and getchangedtargetsandedges.go drop ~400 lines combined while preserving every test-referenced symbol (validateGetChangedTargetsRequest, validateGetChangedTargetsAndEdgesRequest, computeDistances, sendWithDistanceFilter, sendWithDistanceFilterForEdges, buildEdgeSet, transposeOptimizedTarget). Existing tests pass unchanged. Co-Authored-By: Claude Opus 4.7 <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
3 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
The four streaming RPCs in
controller/had grown into ~700-line files thatduplicated the same scaffolding around revision validation, treehash lookup,
cache drains, two-revision graph fetch, name/ID transposition, and
INDIRECT→DIRECT promotion. This PR pulls the shared logic into small,
named helpers and re-expresses each RPC as a top-to-bottom pipeline.
rpc_helpers.go—recordRPCResultdefer used by every RPC;validateRevisionPairshared by both Changed-Targets validators.cache.go— genericcacheReader[T]interface plusreadTreehash,readTreehashPair, andloadCachedResponses[T]unify cache scaffoldingacross the two response types.
graphfetch.go—fetchTwoGraphsconcurrently retrieves both revisionswith sibling cancellation on the first failure.
mappers.go—idMappersbundles the five canonical name→ID mappersand exposes
transpose/chunkMetadata.classify.go—promoteToDirectIfNeededcentralizes theINDIRECT→DIRECT promotion rules.
getchangedtargets.goandgetchangedtargetsandedges.goshrink by ~400lines combined and now read as a sequence of named pipeline steps.
All test-referenced symbols are preserved (
validateGetChangedTargetsRequest,validateGetChangedTargetsAndEdgesRequest,computeDistances,sendWithDistanceFilter,sendWithDistanceFilterForEdges,buildEdgeSet,transposeOptimizedTarget) — no test changes were needed.Test plan
go build ./...go vet ./controller/...gofmt -l ./controller/cleango test ./controller/...go test -race ./controller/...🤖 Generated with Claude Code