diff --git a/internal/lsp/cache/graph.go b/internal/lsp/cache/graph.go
index c1beff8286..d0f80cc401 100644
--- a/internal/lsp/cache/graph.go
+++ b/internal/lsp/cache/graph.go
@@ -11,12 +11,8 @@ import (
 	"golang.org/x/tools/internal/span"
 )
 
-// A metadataGraph holds information about a transtively closed import graph of
-// Go packages, as obtained from go/packages.
-//
-// Currently a new metadata graph is created for each snapshot.
-// TODO(rfindley): make this type immutable, so that it may be shared across
-// snapshots.
+// A metadataGraph is an immutable and transitively closed import
+// graph of Go packages, as obtained from go/packages.
 type metadataGraph struct {
 	// metadata maps package IDs to their associated metadata.
 	metadata map[PackageID]*KnownMetadata
diff --git a/internal/lsp/cache/load.go b/internal/lsp/cache/load.go
index b15c5374e9..6d9c598606 100644
--- a/internal/lsp/cache/load.go
+++ b/internal/lsp/cache/load.go
@@ -93,9 +93,7 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...interf
 
 	if s.view.Options().VerboseWorkDoneProgress {
 		work := s.view.session.progress.Start(ctx, "Load", fmt.Sprintf("Loading query=%s", query), nil, nil)
-		defer func() {
-			work.End(ctx, "Done.")
-		}()
+		defer work.End(ctx, "Done.")
 	}
 
 	ctx, done := event.Start(ctx, "cache.view.load", tag.Query.Of(query))
@@ -241,14 +239,13 @@ func (s *snapshot) load(ctx context.Context, allowNetwork bool, scopes ...interf
 	s.dumpWorkspace("load")
 	s.mu.Unlock()
 
-	// Rebuild the workspace package handle for any packages we invalidated.
+	// Recompute the workspace package handle for any packages we invalidated.
 	//
-	// TODO(rfindley): what's the point of returning an error here? Probably we
-	// can simply remove this step: The package handle will be rebuilt as needed.
+	// This is (putatively) an optimization since handle
+	// construction prefetches the content of all Go source files.
+	// It is safe to ignore errors, or omit this step entirely.
 	for _, m := range updates {
-		if _, err := s.buildPackageHandle(ctx, m.ID, s.workspaceParseMode(m.ID)); err != nil {
-			return err
-		}
+		s.buildPackageHandle(ctx, m.ID, s.workspaceParseMode(m.ID)) // ignore error
 	}
 
 	if len(moduleErrs) > 0 {
diff --git a/internal/lsp/cache/snapshot.go b/internal/lsp/cache/snapshot.go
index 0fa670cf20..87ef595c33 100644
--- a/internal/lsp/cache/snapshot.go
+++ b/internal/lsp/cache/snapshot.go
@@ -1986,13 +1986,7 @@ func (s *snapshot) clone(ctx, bgCtx context.Context, changes map[span.URI]*fileC
 	}
 
 	// Update metadata, if necessary.
-	if len(metadataUpdates) > 0 {
-		result.meta = s.meta.Clone(metadataUpdates)
-	} else {
-		// No metadata changes. Since metadata is only updated by cloning, it is
-		// safe to re-use the existing metadata here.
-		result.meta = s.meta
-	}
+	result.meta = s.meta.Clone(metadataUpdates)
 
 	// Update workspace and active packages, if necessary.
 	if result.meta != s.meta || anyFileOpenedOrClosed {
diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go
index b23ed614f6..9d1ee55810 100644
--- a/internal/lsp/cache/view.go
+++ b/internal/lsp/cache/view.go
@@ -710,8 +710,9 @@ func (s *snapshot) loadWorkspace(ctx context.Context, firstAttempt bool) {
 		scopes = append(scopes, viewLoadScope("LOAD_VIEW"))
 	}
 
-	// If we're loading anything, ensure we also load builtin.
-	// TODO(rstambler): explain the rationale for this.
+	// If we're loading anything, ensure we also load builtin,
+	// since it provides fake definitions (and documentation)
+	// for types like int that are used everywhere.
 	if len(scopes) > 0 {
 		scopes = append(scopes, PackagePath("builtin"))
 	}