You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@texera.apache.org by "juliethecao (via GitHub)" <gi...@apache.org> on 2026/06/12 22:54:07 UTC
[PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
juliethecao opened a new pull request, #5651:
URL: https://github.com/apache/texera/pull/5651
<!--
Thanks for sending a pull request (PR)! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
[Contributing to Texera](https://github.com/apache/texera/blob/main/CONTRIBUTING.md)
2. Ensure you have added or run the appropriate tests for your PR
3. If the PR is work in progress, mark it a draft on GitHub.
4. Please write your PR title to summarize what this PR proposes, we
are following Conventional Commits style for PR titles as well.
5. Be sure to keep the PR description updated to reflect all changes.
-->
### What changes were proposed in this PR?
This PR adds an automatic reviewer suggestion CI job to `.github/workflows/comment-commands.yml`. When a PR is opened or updated (pull_request: opened, synchronize, reopened), the CI automatically runs `git blame -p` at the base commit on each changed file to identify who most recently touched that code. Candidates are split into two groups:
- Committers — collaborators who can be formally review-requested via GitHub's API
- Non-committer contributors — have context but cannot be review-requested; the author can @-mention them to notify
The CI posts a comment in this format:
`Suggested reviewers (based on git blame of changed files):`
`Committers — can be formally requested: @ alice, @ bob`
`Non-committer contributors — cc to notify: @ carol`
`Use /request-review @ alice to request a review, or cc @ carol to notify them.`
On every subsequent push, the job finds the existing suggestion comment (via a hidden HTML marker <!-- texera-reviewer-suggestion -->) and edits it in place, keeping the PR timeline clean. The CI never sends a review request on its own as the author must explicitly use /request-review @ user.
Files with status added are skipped before git blame is attempted since they did not exist at the base commit.
<!--
Please clarify what changes you are proposing. The purpose of this section
is to outline the changes. Here are some tips for you:
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
3. If it is a refactoring, clarify what has been changed.
3. It would be helpful to include a before-and-after comparison using
screenshots or GIFs.
4. Please consider writing useful notes for better and faster reviews.
-->
### Any related issues, documentation, discussions?
Closes #5611
<!--
Please use this section to link other resources if not mentioned already.
1. If this PR fixes an issue, please include `Fixes #1234`, `Resolves #1234`
or `Closes #1234`. If it is only related, simply mention the issue number.
2. If there is design documentation, please add the link.
3. If there is a discussion in the mailing list, please add the link.
-->
### How was this PR tested?
**Unit tests**: 83 tests across 9 suites were written locally (https://github.com/juliethecao/texera/tree/cc-test) to cover the core JavaScript logic extracted from the workflow: `git blame -p` output parsing, candidate ranking, comment body generation, find-or-update marker logic, author/bot exclusion, @ mention parsing, file status filtering, candidate accumulation, and MARKER integrity. Tests were not checked in as the logic lives inside a GitHub Actions script rather than a standalone module.
**Manual CI test**: A test PR was opened on a personal fork (https://github.com/juliethecao/texera/pull/9) against the feature branch as the base. The suggest-reviewers job triggered on open, ran git blame on the changed files, and posted the suggestion comment. Closing and reopening the PR confirmed the comment was updated in place rather than duplicated.
<!--
If tests were added, say they were added here. Or simply mention that if the PR
is tested with existing test cases. Make sure to include/update test cases that
check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how
you tested step by step, ideally copy and paste-able, so that other reviewers can
test and check, and descendants can verify in the future. If tests were not added,
please describe why they were not added and/or why it was difficult to add.
-->
### Was this PR authored or co-authored using generative AI tooling?
Co-authored with Claude Sonnet 4.6 in compliance with ASF guidelines
<!--
If generative AI tooling has been used in the process of authoring this PR,
please include the phrase: 'Generated-by: ' followed by the name of the tool
and its version. If no, write 'No'.
Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
-->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #5651:
URL: https://github.com/apache/texera/pull/5651#discussion_r3426723360
##########
.github/workflows/comment-commands.yml:
##########
@@ -38,12 +47,171 @@ name: Comment commands
on:
issue_comment:
types: [created]
+ pull_request_target:
+ types: [opened, synchronize, reopened]
permissions:
+ contents: read
issues: write
pull-requests: write
jobs:
+ suggest-reviewers:
+ if: github.event_name == 'pull_request_target'
Review Comment:
let's make this new CI separate from `comment-commands` by defining in a new yml file. This is because comment commands is a job triggered by users posting comments, while the `suggest-reviewer` is intended to be triggered whenever the PR is opened, updated (synchronized), or reopened. Mixing two triggers would cause the job being executed more than needed: for example, updating a PR would cause detection on `/take` comment. This won't cause correctness issue because each action would have filters, but it's a waste of CI resource.
##########
.github/workflows/comment-commands.yml:
##########
@@ -38,12 +47,171 @@ name: Comment commands
on:
issue_comment:
types: [created]
+ pull_request_target:
+ types: [opened, synchronize, reopened]
permissions:
+ contents: read
issues: write
pull-requests: write
jobs:
+ suggest-reviewers:
+ if: github.event_name == 'pull_request_target'
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v5
+ with:
+ fetch-depth: 0
+ - uses: actions/github-script@v8
+ with:
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ script: |
+ const { execFileSync } = require('node:child_process');
+ const pull_number = context.payload.pull_request.number;
+ const author = context.payload.pull_request.user.login;
+ const { owner, repo } = context.repo;
+
+ const { data: pull } = await github.rest.pulls.get({ owner, repo, pull_number });
+
+ try {
+ execFileSync('git', ['fetch', 'origin', pull.base.ref], { encoding: 'utf8' });
+ } catch (e) {
+ core.warning(`git fetch for base ref ${pull.base.ref} failed: ${e.message}`);
+ }
+
+ const files = await github.paginate(github.rest.pulls.listFiles, {
+ owner, repo, pull_number, per_page: 100,
+ });
+
+ // Parse `git blame -p` output to find the most-recent commit per file.
+ function latestBlameCommit(blameOutput) {
+ let latest = null;
+ let current = null;
+
+ function finalizeCurrent() {
+ if (!current || current.authorTime == null) return;
+ if (!latest || current.authorTime > latest.authorTime) latest = current;
+ }
+
+ for (const line of blameOutput.split(/\r?\n/)) {
+ const header = line.match(/^([0-9a-f^]+)\s+\d+\s+\d+\s+\d+$/);
+ if (header) {
+ finalizeCurrent();
+ current = { sha: header[1].replace(/^\^/, ''), authorTime: null };
+ continue;
+ }
+ const authorTime = line.match(/^author-time\s+(\d+)$/);
+ if (authorTime && current) current.authorTime = Number(authorTime[1]);
+ }
+
+ finalizeCurrent();
+ return latest;
+ }
+
+ // Count changed files touched per login; track collaborator status.
+ const committerCounts = new Map(); // collaborators
+ const nonCommitterCounts = new Map(); // non-collaborators with a GitHub login
+
+ for (const { filename, status, previous_filename } of files) {
+ if (status === 'removed' || status === 'added') continue;
+ const blamePath = status === 'renamed' ? previous_filename : filename;
+
+ let blameOutput;
+ try {
+ blameOutput = execFileSync(
+ 'git', ['blame', '-p', pull.base.sha, '--', blamePath],
+ { encoding: 'utf8' },
+ );
+ } catch (e) {
+ core.warning(`git blame on ${filename} failed: ${e.message}`);
+ continue;
+ }
+
+ const latest = latestBlameCommit(blameOutput);
+ if (!latest) continue;
+
+ let commit;
+ try {
+ ({ data: commit } = await github.rest.repos.getCommit({ owner, repo, ref: latest.sha }));
+ } catch (e) {
+ core.warning(`Commit lookup for ${latest.sha} failed: ${e.message}`);
+ continue;
+ }
+
+ const login = commit.author?.login ?? commit.committer?.login;
+ if (!login) continue;
+ if (login.toLowerCase() === author.toLowerCase()) continue;
+
+ const loginSource = commit.author?.login ? commit.author : commit.committer;
+ if (loginSource?.type === 'Bot') continue;
+
+ let isCollaborator = false;
+ try {
+ await github.rest.repos.checkCollaborator({ owner, repo, username: login });
+ isCollaborator = true;
+ } catch (_) { /* not a collaborator */ }
+
+ if (isCollaborator) {
+ committerCounts.set(login, (committerCounts.get(login) ?? 0) + 1);
+ } else {
+ nonCommitterCounts.set(login, (nonCommitterCounts.get(login) ?? 0) + 1);
+ }
+ }
+
+ const MAX_EACH = 3;
+
+ const committers = [...committerCounts.entries()]
+ .sort((a, b) => b[1] - a[1])
+ .slice(0, MAX_EACH)
+ .map(([l]) => l);
+
+ const nonCommitters = [...nonCommitterCounts.entries()]
+ .sort((a, b) => b[1] - a[1])
+ .slice(0, MAX_EACH)
+ .map(([l]) => l);
+
+ const MARKER = '<!-- texera-reviewer-suggestion -->';
+
+ let body = `${MARKER}\n`;
+ body += `**Suggested reviewers** (based on \`git blame\` of changed files):\n\n`;
+
+ if (committers.length) {
+ body += `**Committers** — can be formally requested: ${committers.map(l => `\`@${l}\``).join(', ')}\n\n`;
+ } else {
+ body += `**Committers** — none identified\n\n`;
+ }
Review Comment:
Let's make it more readable, suggested format:
```
### Automated Reviewer Suggestions
Based on the git blame history of the changed files, we recommend the following reviewers:
- **Committers with relevant context:** Alice, Bob
You can request their reviews formally with `/request-review @Alice @bob`.
- **Contributors with relevant context:** Carol
You can notify them by mentioning `@carol` in a comment.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang merged PR #5651:
URL: https://github.com/apache/texera/pull/5651
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "Ma77Ball (via GitHub)" <gi...@apache.org>.
Ma77Ball commented on code in PR #5651:
URL: https://github.com/apache/texera/pull/5651#discussion_r3409507283
##########
.github/workflows/comment-commands.yml:
##########
@@ -38,12 +47,170 @@ name: Comment commands
on:
issue_comment:
types: [created]
+ pull_request:
+ types: [opened, synchronize, reopened]
permissions:
+ contents: read
issues: write
pull-requests: write
jobs:
+ suggest-reviewers:
+ if: github.event_name == 'pull_request'
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v5
+ with:
+ fetch-depth: 0
+ - uses: actions/github-script@v8
+ with:
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ script: |
+ const { execFileSync } = require('node:child_process');
+ const pull_number = context.payload.pull_request.number;
+ const author = context.payload.pull_request.user.login;
+ const { owner, repo } = context.repo;
+
+ const { data: pull } = await github.rest.pulls.get({ owner, repo, pull_number });
+
+ try {
+ execFileSync('git', ['fetch', 'origin', pull.base.ref], { encoding: 'utf8' });
+ } catch (e) {
+ core.warning(`git fetch for base ref ${pull.base.ref} failed: ${e.message}`);
+ }
+
+ const files = await github.paginate(github.rest.pulls.listFiles, {
+ owner, repo, pull_number, per_page: 100,
+ });
+
+ // Parse `git blame -p` output to find the most-recent commit per file.
+ function latestBlameCommit(blameOutput) {
+ let latest = null;
+ let current = null;
+
+ function finalizeCurrent() {
+ if (!current || current.authorTime == null) return;
+ if (!latest || current.authorTime > latest.authorTime) latest = current;
+ }
+
+ for (const line of blameOutput.split(/\r?\n/)) {
+ const header = line.match(/^([0-9a-f^]+)\s+\d+\s+\d+\s+\d+$/);
+ if (header) {
+ finalizeCurrent();
+ current = { sha: header[1].replace(/^\^/, ''), authorTime: null };
+ continue;
+ }
+ const authorTime = line.match(/^author-time\s+(\d+)$/);
+ if (authorTime && current) current.authorTime = Number(authorTime[1]);
+ }
+
+ finalizeCurrent();
+ return latest;
+ }
+
+ // Count changed files touched per login; track collaborator status.
+ const committerCounts = new Map(); // collaborators
+ const nonCommitterCounts = new Map(); // non-collaborators with a GitHub login
+
+ for (const { filename, status } of files) {
+ if (status === 'removed' || status === 'added') continue;
+
+ let blameOutput;
+ try {
+ blameOutput = execFileSync(
+ 'git', ['blame', '-p', pull.base.sha, '--', filename],
Review Comment:
Renamed files don't exist at `base.sha`, so blame throws and they're dropped. Use `previous_filename`:
```suggestion
for (const { filename, status, previous_filename } of files) {
if (status === 'removed' || status === 'added') continue;
const blamePath = status === 'renamed' ? previous_filename : filename;
let blameOutput;
try {
blameOutput = execFileSync(
'git', ['blame', '-p', pull.base.sha, '--', blamePath],
```
##########
.github/workflows/comment-commands.yml:
##########
@@ -38,12 +47,170 @@ name: Comment commands
on:
issue_comment:
types: [created]
+ pull_request:
Review Comment:
`pull_request` gives a read-only token for fork PRs, so the comment writes 403. Use `pull_request_target` (as `pr-assignment.yml` does); safe here since the job only blames `base.sha`. Also change line 60 to `'pull_request_target'`.
```suggestion
pull_request_target:
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "juliethecao (via GitHub)" <gi...@apache.org>.
juliethecao commented on PR #5651:
URL: https://github.com/apache/texera/pull/5651#issuecomment-4697191167
| Case | Current behavior | Why |
|---|---|---|
| Adding a new file | `git blame` fails (caught), skips file → "No candidates found" | File didn't exist at base commit — nothing to blame |
| Modifying a file | `git blame` succeeds → suggests committers/non-committers | This is the primary intended use case |
| Deleting a file | Skipped entirely (`status === 'removed'`) → "No candidates found" | File existed at base so blame is technically possible, but see note below |
For all three test PRs, I used `.github/welcome-first-time-contributor.txt` as the test file to add it, modify it, and delete it respectively.
- Add case: https://github.com/juliethecao/texera/pull/10
- Modify case: https://github.com/juliethecao/texera/pull/11
- Delete case: https://github.com/juliethecao/texera/pull/12
Let me know if the modify case should use a more representative file (e.g. something with richer blame history across more contributors).
On the delete case: I currently skip deleted files, but since the file existed at the base commit, `git blame` would technically work. That said, I'm not sure it's the right call as whoever is deleting a file likely already knows what they're doing and is the most appropriate reviewer themselves. Happy to change the logic if you think surfacing blame candidates for deletions would be useful.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "juliethecao (via GitHub)" <gi...@apache.org>.
juliethecao commented on code in PR #5651:
URL: https://github.com/apache/texera/pull/5651#discussion_r3444773010
##########
.github/workflows/comment-commands.yml:
##########
@@ -38,12 +47,171 @@ name: Comment commands
on:
issue_comment:
types: [created]
+ pull_request_target:
+ types: [opened, synchronize, reopened]
permissions:
+ contents: read
issues: write
pull-requests: write
jobs:
+ suggest-reviewers:
+ if: github.event_name == 'pull_request_target'
Review Comment:
- New file `github/workflows/suggest-reviewers.yml` owns the `pull_request_target` trigger and the full `suggest-reviewers` job.
- `comment-commands.yml`: `pull_request_target` trigger removed, `suggest-reviewers` job removed, contents: read permission removed, header blurb about `suggest-reviewers` removed. Now purely `issue_comment` driven as it was originally.
- In `comment-commands.yml`, when `/request-review` is used without any `@mentions`, the previous behavior was a silent `core.warning(...)` that only logs internally and is invisible to the PR author. I replaced it with a visible comment reply pointing the author to the reviewer suggestion comment for candidates (example: https://github.com/juliethecao/texera/pull/15). Happy to revert to the silent log or remove the check entirely if preferred.
##########
.github/workflows/comment-commands.yml:
##########
@@ -38,12 +47,171 @@ name: Comment commands
on:
issue_comment:
types: [created]
+ pull_request_target:
+ types: [opened, synchronize, reopened]
permissions:
+ contents: read
issues: write
pull-requests: write
jobs:
+ suggest-reviewers:
+ if: github.event_name == 'pull_request_target'
+ runs-on: ubuntu-latest
+ steps:
+ - uses: actions/checkout@v5
+ with:
+ fetch-depth: 0
+ - uses: actions/github-script@v8
+ with:
+ github-token: ${{ secrets.GITHUB_TOKEN }}
+ script: |
+ const { execFileSync } = require('node:child_process');
+ const pull_number = context.payload.pull_request.number;
+ const author = context.payload.pull_request.user.login;
+ const { owner, repo } = context.repo;
+
+ const { data: pull } = await github.rest.pulls.get({ owner, repo, pull_number });
+
+ try {
+ execFileSync('git', ['fetch', 'origin', pull.base.ref], { encoding: 'utf8' });
+ } catch (e) {
+ core.warning(`git fetch for base ref ${pull.base.ref} failed: ${e.message}`);
+ }
+
+ const files = await github.paginate(github.rest.pulls.listFiles, {
+ owner, repo, pull_number, per_page: 100,
+ });
+
+ // Parse `git blame -p` output to find the most-recent commit per file.
+ function latestBlameCommit(blameOutput) {
+ let latest = null;
+ let current = null;
+
+ function finalizeCurrent() {
+ if (!current || current.authorTime == null) return;
+ if (!latest || current.authorTime > latest.authorTime) latest = current;
+ }
+
+ for (const line of blameOutput.split(/\r?\n/)) {
+ const header = line.match(/^([0-9a-f^]+)\s+\d+\s+\d+\s+\d+$/);
+ if (header) {
+ finalizeCurrent();
+ current = { sha: header[1].replace(/^\^/, ''), authorTime: null };
+ continue;
+ }
+ const authorTime = line.match(/^author-time\s+(\d+)$/);
+ if (authorTime && current) current.authorTime = Number(authorTime[1]);
+ }
+
+ finalizeCurrent();
+ return latest;
+ }
+
+ // Count changed files touched per login; track collaborator status.
+ const committerCounts = new Map(); // collaborators
+ const nonCommitterCounts = new Map(); // non-collaborators with a GitHub login
+
+ for (const { filename, status, previous_filename } of files) {
+ if (status === 'removed' || status === 'added') continue;
+ const blamePath = status === 'renamed' ? previous_filename : filename;
+
+ let blameOutput;
+ try {
+ blameOutput = execFileSync(
+ 'git', ['blame', '-p', pull.base.sha, '--', blamePath],
+ { encoding: 'utf8' },
+ );
+ } catch (e) {
+ core.warning(`git blame on ${filename} failed: ${e.message}`);
+ continue;
+ }
+
+ const latest = latestBlameCommit(blameOutput);
+ if (!latest) continue;
+
+ let commit;
+ try {
+ ({ data: commit } = await github.rest.repos.getCommit({ owner, repo, ref: latest.sha }));
+ } catch (e) {
+ core.warning(`Commit lookup for ${latest.sha} failed: ${e.message}`);
+ continue;
+ }
+
+ const login = commit.author?.login ?? commit.committer?.login;
+ if (!login) continue;
+ if (login.toLowerCase() === author.toLowerCase()) continue;
+
+ const loginSource = commit.author?.login ? commit.author : commit.committer;
+ if (loginSource?.type === 'Bot') continue;
+
+ let isCollaborator = false;
+ try {
+ await github.rest.repos.checkCollaborator({ owner, repo, username: login });
+ isCollaborator = true;
+ } catch (_) { /* not a collaborator */ }
+
+ if (isCollaborator) {
+ committerCounts.set(login, (committerCounts.get(login) ?? 0) + 1);
+ } else {
+ nonCommitterCounts.set(login, (nonCommitterCounts.get(login) ?? 0) + 1);
+ }
+ }
+
+ const MAX_EACH = 3;
+
+ const committers = [...committerCounts.entries()]
+ .sort((a, b) => b[1] - a[1])
+ .slice(0, MAX_EACH)
+ .map(([l]) => l);
+
+ const nonCommitters = [...nonCommitterCounts.entries()]
+ .sort((a, b) => b[1] - a[1])
+ .slice(0, MAX_EACH)
+ .map(([l]) => l);
+
+ const MARKER = '<!-- texera-reviewer-suggestion -->';
+
+ let body = `${MARKER}\n`;
+ body += `**Suggested reviewers** (based on \`git blame\` of changed files):\n\n`;
+
+ if (committers.length) {
+ body += `**Committers** — can be formally requested: ${committers.map(l => `\`@${l}\``).join(', ')}\n\n`;
+ } else {
+ body += `**Committers** — none identified\n\n`;
+ }
Review Comment:
- Header is now ### Automated Reviewer Suggestions
- Added intro line: "Based on the git blame history of the changed files..."
- Committers bullet: lists them with a second line showing the exact /request-review @Alice @Bob command to copy-paste
- Contributors bullet: lists them with a second line telling the author to mention them in a comment
- Empty state: single bullet "No candidates found from git blame history"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5651:
URL: https://github.com/apache/texera/pull/5651#issuecomment-4696957307
I see on your fork run
```
Suggested reviewers (based on git blame of changed files):
Committers — none identified
No candidates found from blame history.
```
This is not ideal right? for a new file, there is no blame history. will it go to blame the parent folder or related file?
Could you test on three cases:
- adding a new file
- modifying a file
- deleting a file
To see the recommendations of each?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "juliethecao (via GitHub)" <gi...@apache.org>.
juliethecao commented on PR #5651:
URL: https://github.com/apache/texera/pull/5651#issuecomment-4723804075
I tested these two changes (wrapping @mentions in backticks and fixing the pull_request_target job condition) on a test PR within my fork. However, pull_request_target does not fire for same-repo PRs as it only fires when a PR crosses from a fork into an upstream repo, which is the actual use case on apache/texera. To unblock local testing, I created a separate branch (cc-test-both-triggers) with pull_request added as a second trigger so the CI would fire on my fork. This workaround branch is not part of this official PR (which only keeps pull_request_target) and is only for local testing. Test PR results: https://github.com/juliethecao/texera/pull/14
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #5651:
URL: https://github.com/apache/texera/pull/5651#issuecomment-4696184728
<!-- texera-benchmarks-comment -->
## ✅ No material benchmark regressions detected
🟢 12 better · 🔴 0 worse · ⚪ 3 noise (<±5%) · 0 without baseline
> CI benchmark results are noisy; treat <±5% as noise unless repeated.
[Dashboard](https://apache.github.io/texera/dev/bench/) · [Run](https://github.com/apache/texera/actions/runs/27447716932)
| | config | throughput | MB/s | latency | max Δ latest / 7d |
|---|---|---:|---:|---:|---:|
| 🟢 | bs=10 sw=10 sl=64 | 440 | 0.268 | 22,630/28,891/28,891 us | 🟢 -7.5% / 🟢 -19.9% |
| 🟢 | bs=100 sw=10 sl=64 | 967 | 0.59 | 101,588/123,445/123,445 us | 🟢 -23.2% / 🟢 -12.4% |
| 🟢 | bs=1000 sw=10 sl=64 | 1,095 | 0.669 | 920,429/972,469/972,469 us | 🟢 +15.6% / 🟢 +8.5% |
<details><summary>Baseline details</summary>
Latest main `73c76f5` from 2026-06-12T22:35:57.354Z
| config | metric | PR | latest main | 7d avg | Δ latest | Δ 7d |
|---|---|---:|---:|---:|---:|---:|
| bs=10 sw=10 sl=64 | throughput | 440 tuples/sec | 420.77 tuples/sec | 398.12 tuples/sec | +4.6% | +10.5% |
| bs=10 sw=10 sl=64 | MB/s | 0.268 MB/s | 0.257 MB/s | 0.243 MB/s | +4.4% | +10.3% |
| bs=10 sw=10 sl=64 | p50 | 22,630 us | 23,004 us | 24,372 us | -1.6% | -7.1% |
| bs=10 sw=10 sl=64 | p95 | 28,891 us | 31,220 us | 36,082 us | -7.5% | -19.9% |
| bs=10 sw=10 sl=64 | p99 | 28,891 us | 31,220 us | 36,082 us | -7.5% | -19.9% |
| bs=100 sw=10 sl=64 | throughput | 967 tuples/sec | 839.89 tuples/sec | 870.8 tuples/sec | +15.1% | +11.0% |
| bs=100 sw=10 sl=64 | MB/s | 0.59 MB/s | 0.513 MB/s | 0.531 MB/s | +15.1% | +11.0% |
| bs=100 sw=10 sl=64 | p50 | 101,588 us | 119,184 us | 114,334 us | -14.8% | -11.1% |
| bs=100 sw=10 sl=64 | p95 | 123,445 us | 160,656 us | 140,980 us | -23.2% | -12.4% |
| bs=100 sw=10 sl=64 | p99 | 123,445 us | 160,656 us | 140,980 us | -23.2% | -12.4% |
| bs=1000 sw=10 sl=64 | throughput | 1,095 tuples/sec | 948.1 tuples/sec | 1,010 tuples/sec | +15.5% | +8.4% |
| bs=1000 sw=10 sl=64 | MB/s | 0.669 MB/s | 0.579 MB/s | 0.617 MB/s | +15.6% | +8.5% |
| bs=1000 sw=10 sl=64 | p50 | 920,429 us | 1,048,503 us | 994,062 us | -12.2% | -7.4% |
| bs=1000 sw=10 sl=64 | p95 | 972,469 us | 1,107,534 us | 1,046,212 us | -12.2% | -7.0% |
| bs=1000 sw=10 sl=64 | p99 | 972,469 us | 1,107,534 us | 1,046,212 us | -12.2% | -7.0% |
</details>
<details><summary>Raw CSV</summary>
```csv
config_idx,batch_size,schema_width,string_len,num_batches,total_ms,total_tuples,total_bytes,tuples_per_sec,mb_per_sec,lat_p50_us,lat_p95_us,lat_p99_us
0,10,10,64,20,454.86,200,128000,440,0.268,22629.81,28890.58,28890.58
1,100,10,64,20,2067.67,2000,1280000,967,0.590,101588.25,123445.37,123445.37
2,1000,10,64,20,18257.82,20000,12800000,1095,0.669,920429.25,972469.43,972469.43
```
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on PR #5651:
URL: https://github.com/apache/texera/pull/5651#issuecomment-4704222156
@juliethecao let's make sure the suggestion won't actually tag a person. for example in https://github.com/juliethecao/texera/pull/11
if you @Ma77Ball he will receive a github notification. if you `@Ma77Ball` (wrapped in code) he will not receive a notification. let's wrap it in code in suggestion. It is up to the author to actually make the request.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #5651:
URL: https://github.com/apache/texera/pull/5651#issuecomment-4696151233
## [Codecov](https://app.codecov.io/gh/apache/texera/pull/5651?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 52.96%. Comparing base ([`fa5fcbb`](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`894fdab`](https://app.codecov.io/gh/apache/texera/commit/894fdabc5b642c10fba2ff808bdb18bf625defb5?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on main.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #5651 +/- ##
=========================================
Coverage 52.96% 52.96%
Complexity 2523 2523
=========================================
Files 1075 1075
Lines 41743 41743
Branches 4513 4513
=========================================
Hits 22109 22109
Misses 18330 18330
Partials 1304 1304
```
| [Flag](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | | *Carryforward flag |
|---|---|---|---|
| [access-control-service](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.42% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [agent-service](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `34.36% <ø> (ø)` | | |
| [amber](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `53.75% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [computing-unit-managing-service](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `1.65% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [config-service](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.71% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [file-service](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `57.06% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [frontend](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `47.38% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [pyamber](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.72% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [python](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `90.74% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [workflow-compiling-service](https://app.codecov.io/gh/apache/texera/pull/5651/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `58.69% <ø> (ø)` | | Carriedforward from [fa5fcbb](https://app.codecov.io/gh/apache/texera/commit/fa5fcbb60b6f0a305a21635e2560ca0b04b823e2?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
*This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/texera/pull/5651?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "juliethecao (via GitHub)" <gi...@apache.org>.
juliethecao commented on PR #5651:
URL: https://github.com/apache/texera/pull/5651#issuecomment-4755163773
I created another test PR https://github.com/juliethecao/texera/pull/16 that modifies 6 files with different most-recent committers to stress-test the suggestions. However, since this is running on my local fork rather than apache/texera, everyone shows up under Contributors with relevant context. The checkCollaborator API checks collaborators on my fork, so none of the committers from apache/texera are recognized as committers there. On the real repo, they would correctly appear under Committers with relevant context and be formally review-requestable. Once this is officially merged, further testing on apache/texera would be needed to fully validate the committer/contributor split and the MAX_EACH cap under real repo conditions.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(comment-commands): auto suggest reviewers on pr open/update via git blame [texera]
Posted by "Yicong-Huang (via GitHub)" <gi...@apache.org>.
Yicong-Huang commented on code in PR #5651:
URL: https://github.com/apache/texera/pull/5651#discussion_r3448851556
##########
.github/workflows/comment-commands.yml:
##########
@@ -38,12 +47,171 @@ name: Comment commands
on:
issue_comment:
types: [created]
+ pull_request_target:
+ types: [opened, synchronize, reopened]
permissions:
+ contents: read
issues: write
pull-requests: write
jobs:
+ suggest-reviewers:
+ if: github.event_name == 'pull_request_target'
Review Comment:
thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@texera.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org