Implement topic resolver and optimize cache loading by keyurva · Pull Request #1908 · datacommonsorg/mixer

keyurva · 2026-05-22T17:30:42Z

Adds support for the new resolver = "topic" option inside the /v2/resolve API and a parameter to request expanding topic hierarchies.
Integrates dynamic, incremental SV metadata caching into the topic cache manager.
Excludes topics and SVPGs that lack populated list properties in the database.
Prunes dangling parent-child tree references recursively when child targets are empty or skipped.
Bypasses Spanner's edge pagination truncation limit globally by parallelizing multi-node requests in chunked partitions.

Verification / Testing Reference

1. Topic Resolver (expand_topics = false)

Returns immediate sub-topic and variable candidates:

curl -X POST -H "Content-Type: application/json" -H "X-Skip-Cache: true" -d '{
  "nodes": ["dc/topic/Root"],
  "resolver": "topic",
  "expand_topics": false
}' http://localhost:8081/v2/resolve | jq

2. Topic Resolver (expand_topics = true)

Recursively expands the entire nested hierarchy cache:

curl -X POST -H "Content-Type: application/json" -H "X-Skip-Cache: true" -d '{
  "nodes": ["dc/topic/Root"],
  "resolver": "topic",
  "expand_topics": true
}' http://localhost:8081/v2/resolve | jq

3. Embeddings/Indicator Resolver (expand_topics = false)

Performs semantic embeddings search and returns immediate candidates:

curl -X POST -H "Content-Type: application/json" -H "X-Skip-Cache: true" -d '{
  "nodes": ["health"],
  "resolver": "indicator",
  "expand_topics": false
}' http://localhost:8081/v2/resolve | jq

4. Embeddings/Indicator Resolver (expand_topics = true)

Performs semantic search and recursively expands matched topic hierarchies:

curl -X POST -H "Content-Type: application/json" -H "X-Skip-Cache: true" -d '{
  "nodes": ["health"],
  "resolver": "indicator",
  "expand_topics": true
}' http://localhost:8081/v2/resolve | jq

…te topic resolver correctly

gemini-code-assist

Code Review

This pull request introduces a new topic resolver for explicit topic tree navigation and enhances the indicator resolver to support recursive topic expansion. Key changes include the implementation of a TopicExpander interface, a read-through cache for Statistical Variable metadata in TopicCacheManager, and parallelized node fetching in the datasource layer to improve performance and avoid database limits. Feedback focuses on optimizing the cache implementation to prevent stampedes, improving recursive slice allocations, and ensuring robust error handling and logging across the new topic expansion logic.

clincoln8

Thanks Keyur! just a few nits, feel free to ignore

clincoln8 · 2026-05-26T22:25:33Z

 )

+// StatVarInfo stores property metadata for a Statistical Variable.
+type StatVarInfo struct {


do we want description in this?

clincoln8 · 2026-05-26T22:35:27Z

+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+


can you add a file level comment for this one to explain what this adapter is for. if this is intended to only ever be an adapter for the topic fetch, then consider renaming the file to be more specific.

clincoln8 · 2026-05-26T22:37:19Z

@@ -46,7 +46,7 @@
 ) (*pbv2.ResolveResponse, error) {
 	// TODO: Remove this once embeddings search (resolver == "indicator") are


maybe update the TODO to also mention topic resolution being supported in spanner

clincoln8 · 2026-05-26T22:53:56Z

+	ExpandRoots(ctx context.Context, expandTopics bool) ([]*pbv2.ResolveResponse_Entity_Candidate, error)
+	ExpandTopic(ctx context.Context, topicDcid string, expandTopics bool) ([]*pbv2.ResolveResponse_Entity_Candidate, error)
+	GetTopicDisplayName(ctx context.Context, topicDcid string) string
+	GetSVPropertyInfos(ctx context.Context, svDcids []string) (map[string]SVPropertyInfo, error)


nit: thoughts on plural vs singular?

Suggested change

GetSVPropertyInfos(ctx context.Context, svDcids []string) (map[string]SVPropertyInfo, error)

GetSVPropertyInfo(ctx context.Context, svDcids []string) (map[string]SVPropertyInfo, error)

clincoln8 · 2026-05-26T23:34:47Z

+			Node: node,
+		}
+
+		candidates, err := topicExpander.ExpandTopic(ctx, node, expandTopics)


just to confirm, if one topic fails, then we return error for entire response and no partial response?

clincoln8 · 2026-05-26T23:40:20Z

+
+		candidates, err := topicExpander.ExpandTopic(ctx, node, expandTopics)
+		if err != nil {
+			return nil, status.Errorf(codes.Internal, "Failed to expand topic %s: %v", node, err)


should we log here? or higher up in handler_v2 on error?

clincoln8 · 2026-05-26T23:41:11Z

+	}
+
+	chunks := chunkNodes(nodes, fetchAllChunkSize)
+	responses, err := fetchChunksParallel(ctx, ds, req, chunks, pageSize)


should we add some sort of debug log for how often we're chunking requests?

clincoln8 · 2026-05-26T23:42:16Z

+			return n.GetName()
+		}
+	}
+	return ""


nit: log warning for a missing topic name?

clincoln8 · 2026-05-26T23:50:04Z

 	localResp, remoteResp := <-localRespChan, <-remoteRespChan

 	// Note: merger.MergeResolve handles nil inputs (e.g. error handling or empty) gracefully
 	v2Resp := merger.MergeResolve(localResp, remoteResp)


did you double check that internal/merger/merger.go:MergeResolve correctly merges the resolver=topic case?

particularly recursive merge on children

keyurva added 6 commits May 22, 2026 10:12

Update resolve proto schema for topic expansion and property metadata

366e559

Add topic resolver constant and input validation logic

d00cd62

Implement dynamic incremental svinfo caching in topic cache manager

ec94e4b

Add topic hierarchy expansion algorithms to topic cache manager

50c3cc2

Implement topic resolver handler and wire into server routing

a1ce6a4

Exclude topics / svpgs without list props, optimize NodeFetchAll, rou…

70b9e79

…te topic resolver correctly

keyurva changed the title ~~Implement topic resolver and optimize Spanner cache loading~~ Implement topic resolver and optimize cache loading May 22, 2026

gemini-code-assist Bot reviewed May 22, 2026

View reviewed changes

Resolve staticcheck linter warning and apply code review optimizations

2d79331

keyurva requested a review from clincoln8 May 22, 2026 17:56

clincoln8 approved these changes May 26, 2026

View reviewed changes

clincoln8 reviewed May 26, 2026

View reviewed changes

		@@ -46,7 +46,7 @@
		) (*pbv2.ResolveResponse, error) {
		// TODO: Remove this once embeddings search (resolver == "indicator") are

	GetSVPropertyInfos(ctx context.Context, svDcids []string) (map[string]SVPropertyInfo, error)
	GetSVPropertyInfo(ctx context.Context, svDcids []string) (map[string]SVPropertyInfo, error)

Conversation

keyurva commented May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Verification / Testing Reference

1. Topic Resolver (expand_topics = false)

2. Topic Resolver (expand_topics = true)

3. Embeddings/Indicator Resolver (expand_topics = false)

4. Embeddings/Indicator Resolver (expand_topics = true)

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clincoln8 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

clincoln8 May 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

keyurva commented May 22, 2026 •

edited

Loading

clincoln8 May 26, 2026 •

edited

Loading