feat(cluster): cluster graphs — link multiple repos into one connected graph (+ multigraph mode)#2134
feat(cluster): cluster graphs — link multiple repos into one connected graph (+ multigraph mode)#2134StuartMVG wants to merge 19 commits into
Conversation
…s multi-repo paths Extract two helpers into build.py, both extracted verbatim from existing call sites (no behavior change): - load_graph_json: size cap + legacy "edges"->"links" normalization (Graphify-Labs#738) + coercion of directed/multi inputs to a plain undirected Graph (Graphify-Labs#1606), previously triplicated across merge-graphs, global_graph, and the global-graph loader. - merge_prefixed_into: the external-library dedup-by-label + edge rewiring from global_add (the one existing cross-repo identity behavior). merge-graphs and graphify global now call the shared helpers. Groundwork for cluster graphs, which need the same compose semantics.
…d graph
New `graphify cluster` command family (init/add/remove/locate/build/check/
status). A cluster directory holds a committable cluster.yaml naming member
repos and the cross-repo contracts a single-repo scan can't see: api_call
(-> calls_api edge), shared_resource (-> hub concept node + uses edges),
mirrored_file (-> mirrors), depends_on, references.
Members are identified by git URL; local paths resolve per machine via a
gitignored cluster.local.yaml override, the spec's path hint, then origin-
remote auto-discovery — with a warning when a resolved checkout's origin
contradicts the declared url. Node selectors are {repo, file|label|id}
(file suffix-match preferring the file node; label exact then normalized;
id = member-local id), so specs never reference raw prefixed graph ids.
Build composes member graphs under repo_tag:: namespaces (reusing
prefix_graph_for_global + merge_prefixed_into) and resolves declared links
into EXTRACTED edges tagged origin=cluster_spec, writing a standard
graphify-out/graph.json so query/path/explain/affected/export work on a
cluster unchanged. Also writes cluster-manifest.json (hash-based skip of
unchanged rebuilds) and CLUSTER_REPORT.md (every resolved/skipped link).
`cluster check` dry-runs resolution and exits 1 on errors for CI.
affected now traverses the new calls_api/mirrors relations so impact
analysis crosses repo boundaries; depends_on deliberately stays out of the
defaults (it predates clusters and would change single-repo behavior).
Not community detection — that remains cluster-only/cluster.py; help text
and docs disambiguate.
…and affected traversal 48 tests across five files, tmp_path mini-repo fixtures throughout: - test_cluster_spec: spec load/save round-trip, duplicate/reserved/invalid tags, unknown link types, schema_version guard, git-URL normalization equivalences (https/ssh/.git), origin_url from .git/config, and the full path-resolution ladder (local override -> spec path -> origin-remote discovery, mismatch warnings, unresolvable -> None). - test_cluster_build: repo_tag:: prefixes + repo/local_id attrs, externals merged by label with edges rewired (and the auto_links.externals=false opt-out), hash-based rebuild skip, idempotent rebuild after member change, actionable errors for missing graphs/unresolvable members, manifest + CLUSTER_REPORT.md contents, strip_cluster_artifacts. - test_cluster_links: file/label/id selectors (file-node preference, normalized-label fallback), ambiguity errors listing candidates, all on_missing modes, shared_resource hubs, mirrors direction attr, dry-run non-mutation, edge attribute contract (relation/confidence/ origin/_src/_tgt/source_file). - test_cluster_cli: init/add/remove/locate/build/check/status flows, exit codes, .gitignore seeding, remove blocked while links reference a member, dispatch_command routing. - test_affected_cluster_relations: affected crosses repo boundaries via calls_api and mirrors; depends_on stays out of the defaults.
…hangelog README gains a "Cluster graphs (multi-repo)" section with the cluster.yaml format, selector and direction semantics, the per-machine path-resolution story, and a worked command flow; command-reference and common-commands blocks list the new subcommands. ARCHITECTURE.md adds the cluster_graph/ cluster_cli module rows (noting the deliberate naming split from cluster.py's community detection). CHANGELOG entry under 0.9.22.
…pos on build `cluster build` now leaves a back-reference in every resolved member's graphify-out/ so tooling running INSIDE a member knows the repo belongs to a multi-repo cluster. The marker is committable — graphify-out/ travels with the member repo — so it carries no absolute paths: cluster name, the cluster directory's own origin URL (how another dev fetches it), this member's tag, the full member roster (tag + url each), built_at, and a relative dir_hint that fails soft on machines with a different layout. New stdlib-only module graphify/cluster_ref.py (the hook path must never pay the networkx import cost): load_cluster_ref (never raises — None on missing/ corrupt/oversized/future-version markers), cluster_hint_line, unresolvable_message (clone-URL instructions, or init instructions when the cluster has no remote), and resolve_cluster_dir (relative dir_hint verified against the spec name, then parent-sibling discovery scan). Writer semantics: markers are refreshed on every successful build (opt out with `cluster build --no-refs`); a skipped unchanged rebuild still backfills markers that don't exist yet (a freshly cloned member, or a branch switch that removed the untracked file) without churning existing ones; write failures are warnings, never build errors. `cluster remove` deletes the member's marker when its checkout resolves, with a soft note otherwise.
…le-node ID spec
The file-selector ambiguity tiebreak preferred the node whose label equals
the file's basename — true for AST file nodes, false for semantic (LLM)
extractions, which relabel file nodes descriptively ("PR Summary Generator").
Validated on a real 8-repo cluster: re-extracting members with full LLM
graphs turned six previously-clean file selectors ambiguous.
The tiebreak now checks the deterministic signal first: the file-node ID
spec (Graphify-Labs#1504) says a file node's id is normalize_id of the repo-relative path
minus extension, which holds regardless of labeling. Suffix-style selectors
that can't reproduce the full path are covered by round-tripping each
candidate's own source_file through the same rule. The basename-label
heuristic remains as the fallback.
…y surfaces
Inside a member repo (detected via graphify-out/cluster-ref.json):
- query/path/explain/affected accept a bare `--cluster` flag that resolves
the marker to the local cluster directory and runs against its graph.
Mutually exclusive with --graph. Failure modes are loud and actionable,
distinguishing marker-missing, marker-unreadable, cluster-not-local
(with the marker's clone URL, or init instructions when it has no
remote), and cluster-found-but-never-built.
- No-match failures on the DEFAULT graph gain a one-line hint ("this repo
is member 'X' of cluster 'Y' (N members) — re-run with --cluster"):
path source/target/no-path, explain no-match, and affected's
no-unique-match / no-affected-nodes outputs. Explicit --graph/--cluster
runs never hint; a missing or corrupt marker silently hints nothing.
- The hook-guard search nudge and the Gemini BeforeTool nudge append the
same membership line at emit time (the payloads are pre-serialized
constants, so they are rebuilt only when a marker exists — byte-identical
output otherwise). The stdlib-only marker reader keeps the hook path free
of networkx imports.
- The MCP server's no-match answers (get_node, explain, shortest_path)
carry an adapted note (MCP has no --cluster; it points at the cluster
directory), computed per call since the active graph rebinds per
project_path.
… MCP note 22 tests: marker roster/portability (no absolute-path substrings, relative dir_hint), cluster_url from the cluster dir's origin, skip-branch backfill that recreates deleted markers without churning existing ones, --no-refs, remove cleanup (and the soft note for unresolvable members), fail-soft load_cluster_ref (missing/corrupt/non-dict/future-version), dir-hint and discovery resolution incl. moved clusters and name mismatches, --cluster end-to-end through dispatch_command from a member CWD (plus mutual exclusion with --graph, missing/corrupt marker errors, clone-URL message, unbuilt-cluster error), hints present on path/explain/affected failures and absent without or with a corrupt marker or an explicit --graph, hook nudge gaining the membership line while staying valid JSON (byte-identical without a marker), and the MCP get_node no-match note.
…changelog The skill bodies gain a "Cluster member?" paragraph telling assistants to check for graphify-out/cluster-ref.json and reach for --cluster (or the cluster directory) on cross-repo questions, with the fail-safe story when the cluster isn't local. Applied at the source: the skillgen core fragment (after the fast-path paragraph) and the aider/devin monoliths (their query sections — they lack the fast-path anchor), regenerated + blessed, with a sanctioned-diff predicate so the monolith round-trip guard recognizes the paragraph. README documents member back-references (--cluster, --no-refs, the committed-marker vs gitignored-cluster-output asymmetry, last-build-wins for multi-cluster members); CHANGELOG extends the 0.9.22 cluster entry.
…order
Two review findings from the cluster-refs batch:
- __main__.py printed the `cluster <subcommand>` help entry twice, and the
first copy was wedged between merge-graphs' options and clone's
--branch/--out flags, orphaning them from their command. The stray entry
is removed; the surviving one (next to the other multi-repo commands)
gains the cluster-only disambiguation line the deleted copy carried.
- The skill bodies' "Fast path — existing graph" paragraph ("run
`graphify query` immediately") preceded the "Cluster member?" paragraph,
so an agent following the instructions in order would fire a local query
before learning the question may need --cluster. The cluster-member
check now comes first and states the routing rule explicitly (cross-repo
questions run the fast-path query WITH --cluster; single-repo questions
keep the plain local commands), and the fast-path sentence carries the
--cluster clause. Fixed in the skillgen core fragment and regenerated +
blessed; the aider/devin monoliths are unchanged (their paragraph
already lives inside their query section, so no bypass existed there).
A third suggestion — making the monolith-roundtrip predicate for the
cluster paragraph added-only — was declined: the roundtrip's baseline is a
pinned pre-feature v8 SHA that can never contain the paragraph, so the
removed-line case is unreachable, and paragraph deletion is already caught
by skillgen --check against committed artifacts and expected/ snapshots.
…den cluster add Three fixes to the cluster-graphs feature: - apply_spec_links could silently overwrite an existing edge: the cluster graph is a simple Graph (one edge per node pair), so a declared link landing on an already-connected pair replaced that edge's attributes with no indication — clobbering a composed member edge, or an earlier spec link. Occupied pairs are now tracked (pre-seeded from every composed edge, maintained through dry-run) and a collision is a hard error naming both relations, failing `cluster build` and exiting 1 from `cluster check`. README documents the one-relation-per-pair constraint and points at shared_resource hubs for multi-relation contracts. - `cluster build --no-links` after a linked build hit the hash-based skip (spec and member hashes unchanged) and returned "skipped", leaving the linked graph on disk. The manifest now records links_enabled and the skip check compares it; a legacy manifest without the key rebuilds once. - `cluster add` validated the member tag by saving the spec and then re-loading it, so an invalid tag (e.g. --as 'bad::tag') was written to disk before the command failed, corrupting cluster.yaml. Validation now runs before anything is persisted (validate_member_tag, shared with load_spec). The member path is also stored relative to the cluster dir instead of absolute, so a committed spec stays machine-portable — computed with abspath on BOTH sides (never resolve): the hint is later re-joined against the unresolved cluster dir with normpath, so a symlink-resolved base would climb into the wrong tree (e.g. macOS /tmp -> /private/tmp); cross-drive Windows paths fall back to absolute.
…nking
Address the cluster feature's main limitations in four areas:
- Multi-cluster membership: cluster-ref.json becomes a collection
({"version": 1, "clusters": [...]}) with one entry per cluster; each build
upserts only its own entry and `cluster remove` deletes only its own.
query/path/explain/affected accept `--cluster NAME` (and --cluster=NAME);
a bare --cluster on a multi-membership repo fails with the available
names. Genuine name collisions (same cluster name, different git remote)
are rejected by a pre-write conflict check that runs before any build
output lands, so the failure is clean and sticky; a dir_hint mismatch
alone (moved cluster, different checkout layout) warns and refreshes the
marker instead of erroring. The draft single-membership format is a clean
break: regenerate markers with `cluster build`.
- Parallel relationships: `graph_mode: "multi"` clusters and
`graphify extract --multigraph` build keyed MultiDiGraphs on stable
content-derived edge keys (reusing the Graphify-Labs#956 capability probe). Keys
survive build/merge, global add, community detection (parallel weights
aggregate), affected traversal (all matched relations surface), diffing
(occurrence-level identity on multigraphs only; simple graphs keep
(u, v, relation) so attribute churn doesn't report edge changes), the
MCP server, watch mode, and GraphML/Cypher/Canvas/HTML exports (Cypher
MERGEs on graphify_key so re-runs stay idempotent). Incremental extracts
infer the mode from the existing graph, and the no-change early exit
still fires unless a simple->multi conversion is pending.
- Automatic package linking: `auto_links.packages` connects a member's
manifest dependencies to their unique provider in another member via
PEP 503-normalized package identities; ambiguous providers and
already-linked pairs are warned and skipped, declared links win, and
member graphs predating dependency_keys get an actionable warning.
- JSON-first configuration: new specs and local overrides are written as
cluster.json / cluster.local.json with no PyYAML dependency; existing
YAML specs remain readable and are preserved on save.
…igraph conversion Two review fixes: - serve: cluster-ref.json is a committed file, so its fields are untrusted input to assistant-facing output. The MCP no-match cluster note now passes self_tag, cluster_name, member_count, and the multi-cluster name list through sanitize_label, matching every other non-literal field the server emits. - extract --no-cluster --multigraph: a format-conversion run bypasses the no-change early exit, but serialized only that run's incremental extraction — empty when nothing changed — wiping the existing graph (repro: 5 nodes -> 0). The conversion now carries the existing payload forward, filtered like build_merge's prune set: entries owned by sources re-extracted this run (fresh results replace them; carrying both left stale nodes and duplicate parallel keyed edges), deleted sources (matched by raw, relative, and posix spellings), and newly-excluded sources. The in-memory core of _prune_graph_json_sources is extracted as _filter_payload_sources and shared by both call sites. Tests: conversion of an unchanged graph preserves node ids, normalized link signatures (endpoints + relation, ignoring only the assigned key), and a seeded hyperedge's content; conversion coinciding with a changed file drops the file's stale nodes and produces no duplicate parallel edges while other files' content survives.
…ec and link resolution - Compose member graphs into a directed graph in both modes and load members with directed=True: the undirected round-trip re-emitted edge endpoints by node insertion order, silently flipping caller/callee in the written cluster graph (Graphify-Labs#760 class) — affected/path/explain force directed=True at load, so the flip inverted traversal. _src/_tgt markers are popped at write like export.to_json. - direction: "both" now materializes a real reverse edge (traversal reads topology, not attrs) and the enum is validated; the declared link still owns the unordered pair in simple mode. - _norm_source_file: removeprefix, not lstrip (".env" was matched as "env"). - Corrupt member graph.json raises an actionable ClusterSpecError naming the member instead of a raw JSONDecodeError; cluster check reports it. - Refuse self-composition (cluster dir as its own member) and empty-member builds; URL-less clusters no longer bypass the marker conflict check. - Re-enumerate member community ids at compose so unrelated "community 0" groups don't merge across repos. - Label selectors fall back to cluster-wide externals, so a selector naming any referencing member survives spec reordering. - Gate the O(E) json.dumps edge-sort tiebreak to multigraph builds; simple builds keep upstream's sort and tie behavior. - Remove dead strip_cluster_artifacts.
…p routing, sanitized hook context - extract reads the existing graph's multigraph flag on EVERY rebuild, so --force without a repeated --multigraph no longer silently downgrades the format; --no-multigraph is the explicit downgrade path (warns that parallel relations collapse) and conversion now bypasses the no-change early exit in both directions. - cluster subcommand flags that need a value hard-error when it's missing (cluster init --name used to mkdir a directory literally named --name), and unknown --options are rejected instead of becoming positionals. - Help tokens anywhere in a cluster invocation print USAGE and exit 0 — cluster add --help can no longer fall through to a handler; requested help goes to stdout. The main --help documents --cluster [NAME] and --multigraph/--no-multigraph. - cluster add refuses the cluster directory as its own member. - The search-nudge hook sanitizes cluster-ref.json fields (committed marker = untrusted input to assistant-facing context) and coerces member_count. - query gains the same no-match cluster-membership hint as path/explain/ affected, matching what the hook text promises.
…ts, honest affected fallback - MCP get_neighbors iterates edge_datas (one line per parallel edge, like the query surface): edge_data picked one arbitrary parallel edge, so a relation_filter could return empty for relations that exist. Extracted as serve._neighbor_lines for testability. - MCP query_graph appends the cluster-membership note on no-match, matching the other tools. - cluster_ref hint/error builders sanitize marker fields (cluster_name, self_tag, cluster_url) and coerce member_count — the marker travels with clones and is untrusted; security.py is stdlib-only so the lazy import keeps the hook path light. - affected's location gate is via_location again: an edge with source_file but a null source_location (allowed by the extraction schema) rendered a non-clickable "file:-" instead of falling back to the node's def line.
…both" and multigraph stickiness The cluster bullet sat under the already-shipped 0.9.22 heading (the branch was rebased across three releases); it now lives under Unreleased, split into per-feature bullets that match the planned PR partition. README documents the implemented direction: "both" semantics, multigraph format persistence and --no-multigraph, and cluster-wide external selector resolution.
…raphs and specs, deterministic simple-edge collapse - The search-nudge hook no longer interpolates ANY marker-controlled text: sanitize_label strips control chars and caps length, but attacker-chosen words still read as instructions in automatically-injected assistant context. Hook output is now static guidance plus a bounds-checked member count (1..100_000, bools rejected); user-invoked hints and errors keep showing sanitized names. select_cluster_ref accepts the cleaned name too, so a name copied from a hint always selects. - load_graph_json validates structure (nodes list with hashable ids, links with hashable source/target) and wraps every failure in a ValueError naming the path — valid-JSON-but-not-a-graph inputs used to escape as raw KeyError/TypeError tracebacks through cluster build, merge-graphs, and the global graph. - Cluster specs report JSON line/column and YAML parse errors as ClusterSpecError, and field shapes are validated (members/links lists, defaults/auto_links mappings, auto_links.* booleans). - _filter_payload_sources drops hyperedges whose member nodes were pruned, so a multigraph conversion carry-forward can't leave dangling hyperedge references. - The edge-sort content tiebreak now also applies to simple-graph edges with a DUPLICATED (source, target, relation) identity: semantic chunks complete in nondeterministic order, so which duplicate won the collapse varied run-to-run. Unique edges (the typical case) still skip the json.dumps cost. - --multigraph and --no-multigraph together are a usage error (exit 2); repeating a flag stays idempotent.
|
Additional real-world validation for the multigraph mode from an Expo / React Native TypeScript corpus on graphifyy 0.9.25. A fresh code-only extraction reports exactly two same-endpoint relation collisions:
Both relations are semantically useful; a simple graph retains only one edge per pair. This corpus is a compact additional regression case for |
…site Shape reported from a real Expo/React Native corpus on PR Graphify-Labs#2134: a component both calls and indirect_calls the same target. explain must render both keyed edges, each citing its own relation site (L45/L63), not one arbitrary edge — the last query surface without a dedicated parallel-relations test.
Thank you. These are excellent real-world examples of the exact information-loss case --multigraph is intended to solve. Both relation pairs map to distinct keyed edges on this branch, and the query/explain paths display parallel relations independently. I’ve added a focused explain regression test based on your calls + indirect_call example. It verifies that both edges are rendered with their respective relation sites (L45 and L63). If you’re willing to test the PR branch against the full corpus, that would be very useful. Please test at 36ecb25 so the results are comparable. The most valuable checks would be:
No need to share the corpus. The output snippets and before/after edge counts would be enough. Thanks again for the concrete case. |
Summary
This PR adds two related graph capabilities:
It also makes member repositories cluster-aware, so existing
query,path,explain, andaffectedcommands can discover and use their cluster graph.Unlike
merge-graphsand the global graph, which union repositories side by side, a cluster graph explicitly models connections such as API calls, shared resources, mirrored files, dependencies, and references.Quick start
Each member repository must have an existing graph:
Create and build the cluster:
Declare links in
cluster.json:{ "links": [ { "type": "api_call", "name": "events-api", "from": { "repo": "frontend", "file": "src/lib/events-client.ts" }, "to": { "repo": "backend", "file": "src/routes/events.ts" } } ] }Then validate and build:
The resulting
graphify-out/graph.jsonworks with existing commands:From inside a member repository:
Cluster graphs
A committable
cluster.jsondefines members and cross-repository links.Git URLs are used as portable member identities when available. Checkouts resolve per machine through:
cluster.local.jsonoverrideURL-less repositories remain supported through path hints and local overrides.
Members are composed under
tag::namespaces into a directed graph. Link selectors use{repo, file|label|id}:filesuffix-matchessource_filelabelmatches exactly, then case-insensitivelyidaccepts a member-local node IDUsers do not need to write the namespace-prefixed IDs found in the composed graph.
Supported links include:
api_callshared_resourcemirrored_filedepends_onreferencesdirection: "both"materializes a real reverse edge rather than storing direction as metadata only.auto_links.packagesconnects manifest dependencies to their unique provider in another member repository. Ambiguous providers are reported and skipped, and declared links take precedence.cluster checkperforms the same spec, member, and selector validation as a build without writing output. It exits nonzero on errors, making it suitable for CI.Member awareness
cluster buildwrites a portablegraphify-out/cluster-ref.jsonmarker into each member repository. This can be disabled with--no-refs, andcluster removeremoves only its own membership entry.The marker contains no absolute paths and can travel with a clone. It enables:
query,path,explain, andaffectedwith--cluster [NAME]Missing or corrupt markers never break ordinary queries or hooks.
Markers are treated as untrusted input. Automatically injected hook context uses static guidance plus a bounds-checked member count; user-invoked diagnostics sanitize and bound marker-controlled text.
Writing markers into member worktrees is intentionally opt-out.
cluster initseparately adds the cluster directory's generatedgraphify-out/to.gitignore.Multigraph mode
graphify extract . --multigraphMultigraph builds use keyed
MultiDiGraphedges so parallel relations between the same nodes survive through:affectedEdge keys are stable and content-derived. Cypher exports include
graphify_keyinMERGEoperations to remain idempotent.The graph format is sticky across rebuilds, including
--force. Converting back requires an explicit command:graphify extract . --no-multigraphThat conversion warns that parallel relations will collapse. Conversion in either direction carries the existing graph forward.
Simple graphs retain their existing edge order. A content tiebreak is applied only where multiple simple edges have the same
(source, target, relation)identity, preventing run-to-run changes in which duplicate survives.Community detection aggregates parallel edge weights before running algorithms that require a simple graph.
Impact analysis
affectednow traverses the cluster-specificcalls_apiandmirrorsrelations by default.depends_onremains opt-in to avoid changing the established default traversal behavior for existing single-repository graphs.Parallel matching relations are all surfaced when running against a multigraph.
Failure behavior
Validation occurs before writing cluster output. Actionable errors are produced for:
External-library selectors and community IDs are normalized across members so results do not depend on member order or accidentally merge unrelated communities.
Directed composition is load-bearing: converting member graphs through an undirected intermediate can reverse caller/callee endpoints based on node insertion order. Regression coverage pins this behavior.
Review map
graphify/build.py,graphify/global_graph.pygraphify/cluster_graph.py,graphify/cluster_cli.pygraphify/cluster_graph.pygraphify/cluster.py(unrelated to cluster specs, despite the name)graphify/cluster_ref.py,graphify/cli.py,graphify/serve.py--cluster, hints, and sanitizationgraphify/build.py,graphify/analyze.py,graphify/affected.pygraphify/export.pygraphify/manifest_ingest.pygraphify/skill*.md,tools/skillgen/tests/test_cluster*.py,tests/test_multigraph_build.pyThe first commit centralizes graph JSON loading and namespace-prefixed merging for
merge-graphs, the global graph, and clusters. It is intended to preserve successful-path behavior while normalizing malformed-input handling into clean CLI errors.Additional fix
watchrebuilds now passroot=project_roottobuild_from_json, aligning watch-generatedsource_filepaths with those produced bygraphify build(extends the #932 path normalization, which previously covered only the build/extract path, to watch rebuilds).This fix is independent of clusters and can be split into a separate PR if maintainers prefer stricter scope.
Related issues and discussions
api_calllinks connect client call sites to backend handlers, andaffectedtraverses those links across repository boundaries. The issue's proposed automatic framework-specific route extraction, API-contract matching, and dangling-contract detection remain future work.connectoperation is not included.merge-graphsas a namespaced cross-repository union. Cluster graphs retain namespacing while adding explicit edges between members.Direction-preservation coverage follows the regression history in #760, #1061, and #1174. Those issues are already closed; they are cited as design and regression context rather than as issues resolved by this PR.
Testing
At
4ade974:--check--audit-coverage--schema-singleton--monolith-roundtrip--always-on-roundtripCoverage includes spec validation, directed composition, selector resolution, member-order independence, bidirectional links, marker lifecycle and sanitization, all four
--clusterquery surfaces, MCP and hook hints, stable multigraph keys, format conversion in both directions, mixed-mode composition, affected traversal, and exporter round-trips.Related PRs
Supersedes the earlier stacked drafts #2120, #2121, and #2122, consolidating the same work into a single review.