but I feel unqualified to review this, so it might be good to have someone else review it as well?

Reviewed 8 of 8 files at r1, all commit messages.
Reviewable status: 2 unresolved discussions, needs platform reviewer assigned, needs at least two assigned reviewers

+@cohnt for an additional feature review, please.
FTR -- the role of the feature reviewer is to make sure that the changes are good for the feature (here GCS / ImplicitGCS). The additional platform review will be able to provide some backup on e.g. the c++ implementation.

Reviewable status: 2 unresolved discussions, LGTM missing from assignee cohnt, needs platform reviewer assigned

geometry/optimization/graph_of_convex_sets.cc line 537 at r1 (raw file):

Done. My thinking was that since the ell variables were strictly internal, there was zero risk of using the same variables. But I think you're right that it's just cleaner to make new variables here, too.

geometry/optimization/test/implicit_graph_of_convex_sets_test.cc line 226 at r1 (raw file):

It's the job of the tests in graph_of_convex_sets_test to confirm that the AddFromTemplate methods do the right thing. The tests in this file can assume that this part of the implementation is correct, and must test the new logic (e.g. that we call the AddFromTemplate methods the correct number of times)

Mostly looks good. My only concern is related to the determinism of the order vertices and edges are added, since they're internally being stored in std::map objects. (Presumably, this could lead to nondeterminism downstream, which I'm guessing we want to avoid. For example, in breaking ties when expanding vertices in GCS*.) If this isn't a problem, or if I'm misunderstanding what std::map does behind the scenes, then feel free to ignore.

Reviewed 5 of 8 files at r1, 3 of 3 files at r2, all commit messages.
Reviewable status: 6 unresolved discussions, LGTM missing from assignee cohnt, needs platform reviewer assigned

geometry/optimization/test/graph_of_convex_sets_test.cc line 116 at r2 (raw file):

  EXPECT_EQ(v->ambient_dimension(), 3);
  EXPECT_EQ(v->name(), "point");

btw do we need test coverage for the case where multiple vertices have the same name? Ditto below for edges

geometry/optimization/graph_of_convex_sets.h line 687 at r2 (raw file):

  (but not any edges) from `v`. `v` does not need to be registered with this GCS
  instance; this method can be used to effectively copy a Vertex from another
  GCS instance into `this`. */

nit: This new vertex will now have a new VertexId, correct? Might be useful to document that this is also different.

Suggestion: Adds a new vertex (and assigns a new unique VertexId).

geometry/optimization/graph_of_convex_sets.h line 706 at r2 (raw file):

  @throws std::exception if `u` or `v` do not match the sizes of the
  `template_edge.u()` and `template_edge.v()` vertices.
  @throws std::exception if edges have slack variables. We can add this support

nit: "We can add this support once it's needed." Should probably be in a TODO, rather than in the docstring?

geometry/optimization/graph_of_convex_sets.h line 711 at r2 (raw file):

  Edge* AddEdgeFromTemplate(Vertex* u, Vertex* v, const Edge& template_edge);

  /** Returns the first vertex with the given name, or nullptr if no such vertex

What does "first" mean in this context? Since the vertices are stored internally as a map, I think this method might not be deterministic.

Ditto for GetMutableVertexByName, GetEdgeByName, and GetMutableEdgeByName