feat(tgb): add AllTargetsFileHashes column 22 support - #285
Open
yushan8 wants to merge 7 commits into
Open
Conversation
Add TGB support for the AllTargetsFileHashes sidecar so the AllTargetsFiles feature works regardless of graph_format. Column 22 stores a length-prefixed sequence of (key, value) string pairs, zstd-compressed. Old readers skip unknown column IDs; new readers on old blobs get nil — no format version bump needed. The TGB comparison path in the controller now checks AllTargetsFileHashes via a cheap single-column decode before the full columnar diff, matching the gob path's behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Verify that AllTargetsFileHashes survives TGB encode/decode via both full Decode and the lazy Reader.AllTargetsFileHashes() accessor. Covers present, absent, and empty-map cases. Also fixes canonicalise() to preserve AllTargetsFileHashes so existing round-trip tests remain correct when graphs carry the sidecar. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
yushan8
force-pushed
the
yushan/all-targets-config-v2
branch
from
August 19, 2026 16:49
dc9447b to
45cd0c3
Compare
MergeChunks was not merging AllTargetsFileHashes from metadata chunks, so the field was silently dropped before TGB encoding. This caused the AllTargetsFiles trigger to never fire on the TGB comparison path. Also adds controller-level TGB tests for both trigger and no-trigger cases, verifying end-to-end that AllTargetsFileHashes survives the full write → read → compare pipeline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
5 tasks
yushan8
changed the title
Map iteration order is random in Go, so the same logical AllTargetsFileHashes could produce different byte sequences across runs, violating the deterministic computation property and causing spurious cache misses. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
gofmt had left ExcludedRegex/UseBzlmod/AllTargetsFiles misaligned. Also adds a unit test exercising the AllTargetsFileHashes extraction in FromProto directly (previously only covered indirectly via mapper tests).
- gofmt alignment in colNames map and a test's Params struct literal - DecodeGraph now reuses Reader.AllTargetsFileHashes() instead of duplicating the column-decode logic inline
yushan8
marked this pull request as ready for review
August 19, 2026 21:55
Makes it clearer that all targets in the graph are reported as changed when the trigger fires, not just the ones that happen to have different hashes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
sywhang
approved these changes
Aug 20, 2026
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
Adds TGB (Target Graph Binary) format support for
AllTargetsFileHashes, stacked on #286.ALL_TARGETS_FILE_HASHES) — length-prefixed key-value string pairs, zstd compressedReader.AllTargetsFileHashes()lazy accessor decodes only column 22 (cheap, no full graph decode)MergeChunkspropagates AllTargetsFileHashes from metadata chunks before TGB encodingFiles changed
internal/tgb/format.go— column ID 22internal/tgb/encode.go—encodeStringMap+ column emissioninternal/tgb/reader.go—decodeStringMap+AllTargetsFileHashes()accessor +DecodeGraphintegrationinternal/tgbdiff/adapt.go—MergeChunkspropagation fixcontroller/getchangedtargets.go—allTargetsChangedFromTGB(), TGB path wiringcontroller/getchangedtargets_tgb_test.go— trigger/no-trigger testsinternal/tgb/tgb_test.go— round-trip testsTest plan
Stack: depends on #286