feat: add JSONL bulk dep add#3530
Merged
Merged
Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
On large dependency graphs (230+ edges), bd dep add performs a full DetectCycles traversal after every insertion — O(V+E) per call. This causes individual dep add calls to hang for 3+ minutes as the graph grows beyond ~150 edges. Add --no-cycle-check to both `dep add` and the `dep --blocks` shorthand. When set, the post-insert warnIfCyclesExist call is skipped. Callers doing bulk wiring can run `bd dep cycles` once at the end to verify the complete graph. The flag is intentionally not the default: single-edge interactive use keeps cycle detection on. Bulk callers opt in explicitly.
94368de to
51aad4e
Compare
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
Fixes #3408.
Adds a machine-friendly bulk form for
bd dep add:bd dep add --file <path>reads newline-delimited JSON dependency edges from a filebd dep add --file -reads the same JSONL format from stdin{ "from": "bd-2", "to": "bd-1", "type": "blocks" }, withtypedefaulting to--typeissue_id/depends_on_idaliases are accepted for machine-generated recordsThe transaction dependency path now exposes
DependencyAddOptionsso bulk--no-cycle-checkskips the per-edge recursive cycle check as well as the post-add warning. Normal transaction adds still use the sharedissueops.AddDependencyInTxvalidation path.Coordination
Preflight found and reviewed overlapping PRs before updating this PR:
rjc123): directly related--no-cycle-checkwork. This PR is logically stacked on the approved perf: add --no-cycle-check flag to bd dep add for bulk wiring #3521 headbb0493d20and should not merge before perf: add --no-cycle-check flag to bd dep add for bulk wiring #3521 lands, unless maintainers intentionally want to merge the stack together while preserving Robin’s commit attribution.I updated this existing PR instead of opening a duplicate PR.
Tests
BEADS_TEST_EMBEDDED_DOLT=1 go test -tags gms_pure_go ./cmd/bd -run '^TestEmbeddedDepBulkNoCycleCheckSkipsPerEdgeCycleValidation$|^TestEmbeddedDep$|^TestEmbeddedDepNoCycleCheck$' -count=1go test -tags gms_pure_go ./cmd/bd -run '^TestDepAddFlagAliases$|^TestDepBlocksFlag$|^TestDepCommandFlags$' -count=1go test -tags gms_pure_go ./internal/storage/issueops ./internal/storage/dolt ./internal/storage/embeddeddoltmake testwithTMPDIR=/var/tmp/beads-test-bd-wjl-review GOTMPDIR=/var/tmp/beads-test-bd-wjl-review