Add description to embeddings - #732
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the embedding generation process in both the aggregation and ingestion helpers to query Spanner directly using GQL and stream the results to BigQuery, replacing the previous external query approach. It also updates the configuration schema to support mapping node types to specific predicates and introduces pandas for parsing statistical variable files. The review feedback highlights several critical issues: a bug where node_types is passed instead of spec to _generate_spanner_query (which bypasses the filter), a potential crash when streaming empty batches to BigQuery, runtime failures when reading gs:// paths with pandas due to a missing gcsfs dependency, swallowed exceptions during job execution, and an unused parameter in _fresh_data_condition.
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Compatibility | 9 high |
| UnusedCode | 1 medium |
| Documentation | 2 minor |
| ErrorProne | 1 high |
| Security | 4 medium |
| CodeStyle | 4 minor |
| Complexity | 1 medium |
🟢 Metrics 0 complexity
Metric Results Complexity 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
| subject_id, | ||
| types, | ||
| value) | ||
| value, |
There was a problem hiding this comment.
We talked about thsi offline, but just a reminder that you cannot modify this file anymore. It's locked! pls talk to @juliawu about how to create a schema migration!
This PR update the ingestion-helper and aggregation-helper to add the structure that read predicate from edge and then related value from the linked Node
It will build JSON structure
{
subject_id: DCID
name: (DCID name field)
properties {
predicate: value0, value1 etc.
...
}
}
and add the description predicate into the embeddings.
Testing of both ingestion-helper and aggregation-helper are done on test DB: https://pantheon.corp.google.com/spanner/instances/dc-kg-test/databases/dc_graph_2026_01_27
DCP test is also done with a newly created DCP instance and related DB: https://pantheon.corp.google.com/spanner/instances/dcp-testing/databases/dev-shixiao-dc-db
ingestion workflow finishes properly in:
https://pantheon.corp.google.com/workflows/workflow/us-central1/dev-shixiao-dc-ingestion-workflow/execution/92fb3d1b-fd20-4753-ac2c-eafb303267b5/summary?project=datcom-website-dev&e=13803378&mods=-monitoring_api_staging
Validated we can properly generate embeddings (if description do not exists, we will embed the subject_id and name only)
Details of the change includes:
update schema to have Graph include the timestamp
change of the config to modify node_type as a dict of {type:[list of predicates]}
update the DDL statement to do graph query to read predicates and related values
update the timestamp filter logic that if any of (source node, predicated node) is updated, we will regenerate the embeddings
update the unit test to compare the full DDL statement with expected value save in files