Skip to content

Conversation

Orbital-Web
Copy link
Contributor

@Orbital-Web Orbital-Web commented May 23, 2025

Description

^
Will be relevant even more once kg is merged. Lets us filter specific types of PRs and Issues

How Has This Been Tested?

ran on my own repo with prs and issues

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

@Orbital-Web Orbital-Web requested review from Weves and evan-onyx May 23, 2025 23:11
@Orbital-Web Orbital-Web requested a review from a team as a code owner May 23, 2025 23:11
Copy link

vercel bot commented May 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
internal-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 3, 2025 11:38pm

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Updates GitHub connector metadata handling by adding comprehensive PR and issue tracking fields with proper timezone management in backend/onyx/connectors/github/connector.py.

  • Added critical PR metadata fields (assignee, timestamps, merger info) with UTC timezone standardization
  • Added issue tracking metadata (assignee, timestamps, labels) for better issue context
  • Implemented None-value filtering in metadata using dictionary comprehension
  • Maintains backward compatibility while enhancing data completeness
  • Tested functionality with real-world PR and issue scenarios

1 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings | Greptile

assert {
item.semantic_identifier for item in cast(list[Document], outputs[1].items)
} == {"PR 3 Repo 2", "PR 4 Repo 2"}
} == {"3: PR 3 Repo 2", "4: PR 4 Repo 2"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is here because we add the pr/issue number to the semantic id (seems a little unnecessary here, but normally the title of the issue/pr wouldn't include the number).

This will be important for kg because we want to be able to search for the entity "4: Fixed ABC", when the user asks for PR 4.

Copy link
Contributor

@evan-onyx evan-onyx left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM with minor nits!

@Orbital-Web Orbital-Web added this pull request to the merge queue Jun 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jun 4, 2025
@Orbital-Web Orbital-Web added this pull request to the merge queue Jun 4, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jun 4, 2025
* feat: updated github metadata

* feat: nullity check

* feat: more metadata

* feat: userinfo

* test: connector test + more metadata

* feat: num files changed

* feat str

* feat: list of str
Merged via the queue into main with commit 4bb3ee0 Jun 4, 2025
11 checks passed
@Orbital-Web Orbital-Web deleted the connector-metadata branch June 4, 2025 19:48
ZhipengHe pushed a commit to ZhipengHe/onyx that referenced this pull request Jun 6, 2025
* feat: updated github metadata

* feat: nullity check

* feat: more metadata

* feat: userinfo

* test: connector test + more metadata

* feat: num files changed

* feat str

* feat: list of str
AnkitTukatek pushed a commit to TukaTek/onyx that referenced this pull request Sep 23, 2025
* feat: updated github metadata

* feat: nullity check

* feat: more metadata

* feat: userinfo

* test: connector test + more metadata

* feat: num files changed

* feat str

* feat: list of str
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants