Skip to content

feat(ingest): comment app ingest reports on the merged PR / ingest:在已合并的 PR 上评论 app 的 ingest 报告 - #4503

Merged
Oseltamivir merged 2 commits into
mainfrom
feat/ingest-report-callback
Oct 25, 2026
Merged

Oseltamivir merged 2 commits into
mainfrom
feat/ingest-report-callback

Conversation

@Oseltamivir

Copy link
Copy Markdown
Collaborator

Nobody on an InferenceX PR is told when its results land in the database or when the app's ingest fails; checking means opening the InferenceX-app run and grepping logs.

Change

  • New ingest-results-callback.yml (repository_dispatch: ingest-results-completed) runs infx.workflows.ingest_results_callback, mirroring stage-results-callback.yml.
  • The PR is not taken from the payload. The callback loads merge-run-id, requires it to be a merge-ingest.yml push to main, and picks the PR whose merge_commit_sha is that run's head_sha (/commits/{sha}/pulls), i.e. the PR whose changelog entry was published.
  • It posts one comment per merge run with status, links (app ingest run, Merge Ingest run, source sweep run) and a benchmark / eval / trace / power × new / duplicate / skipped / unmapped / failed table, plus unmapped names. It looks for its own <!-- ingest-results-report merge-run=<id> --> comment and PATCHes it, so reruns and failure-then-recovery update in place.
  • Every payload value is validated (source repo, numeric IDs, non-negative integer counts, plain-identifier names) before anything is rendered.
  • Companion: feat(ingest): per-run ingest report and PR callback / ingest:按运行统计的报告与 PR 回调 InferenceX-app#1495 produces the counts and sends the dispatch on success and failure.

Why a callback: the app's INFX_MAIN_PAT only reads artifacts, and the app already uses PAT to dispatch staging callbacks here, which then comment with github.token. Reusing that path needs no new secret or cross-repo write token.

Maintainer action: none beyond merging; land this before the app PR so dispatches have a listener.

AI model disclosure

Prepared with Claude Opus 5.5 (claude-opus-5-5), which wrote the code, tests and description. No delegated agents.

中文

目前 InferenceX PR 在结果写入数据库或 app 端 ingest 失败时都不会收到任何通知,只能打开 InferenceX-app 的运行并 grep 日志。

本 PR 新增 ingest-results-callback.yml,接收 ingest-results-completed 并运行 infx.workflows.ingest_results_callback,模式与 stage-results-callback.yml 一致。PR 编号不取自 payload:回调先确认 merge-run-id 是 merge-ingest.yml 在 main 上的 push 运行,再通过 /commits/{sha}/pulls 找到 merge_commit_sha 与之相同的 PR,即发布了 changelog 条目的 PR。每个合并运行只发一条带标记的评论,包含状态、相关运行链接以及 benchmark / eval / trace / power 的 new / duplicate / skipped / unmapped / failed 统计表;重跑或失败后恢复时原地更新。所有 payload 字段在渲染前都会校验。配套的 app PR 为 SemiAnalysisAI/InferenceX-app#1495。无需新增 secret,请先合并本 PR。

Add ingest-results-callback.yml, which receives ingest-results-completed
from InferenceX-app, resolves the PR merged by the Merge Ingest run's
commit and posts the per-run ingest counts as one marker comment per
merge run, updated in place on reruns and failures.

中文:在已合并的 PR 上评论 InferenceX-app 的 ingest 报告。新增 ingest-results-callback.yml,接收 InferenceX-app 发送的 ingest-results-completed,根据 Merge Ingest 运行的提交解析已合并的 PR,并以每个合并运行一条带标记的评论发布本次 ingest 的统计;重跑或失败时原地更新。
@github-actions

Copy link
Copy Markdown
Contributor

inferencex-e2e/infx diff: +336 -0

Generally most InferenceX submissions don't need to add changes to the infx package. On the off chance they do, generally the diff is small. Please prompt your agent to clean your PR to keep your diff to only necessary & minimal changes too. We have an increased code quality bar for changes to the infx package.

中文

大多数 InferenceX 提交通常不需要修改 infx 包。即使确实需要,改动通常也很小。请让你的 agent 清理 PR,使改动只包含必要且最小的内容。infx 包的改动有更高的代码质量要求。

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Comment on lines +63 to +67
for category in CATEGORIES:
row = counts.get(category)
if not isinstance(row, dict):
raise RuntimeError(f"Callback report is missing {category} counts")
lines.append(f"| {category} | {' | '.join(str(_count(row.get(o))) for o in OUTCOMES)} |")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 (optional) A failed or partial app ingest whose report lacks a category or outcome posts no PR comment, not even the failure notice. This holds only if the app sends such reports. render_report raises when any of the 4 categories or 5 outcomes is missing, or a present detail key is null. Line 132 runs it before anything is posted. Fix: check the report apart from the status, so the status line and run links always post. A report that is missing or malformed should show a 'counts unavailable' note instead of aborting report(). Today only report: null gets that note. [also at: inferencex-e2e/infx/workflows/ingest_results_callback.py:82 - If one unmapped name falls outside the NAME pattern, the PR gets no ingest comment at all, not even the failure notice.]

Why this was flagged

The trigger depends on the companion app (SemiAnalysisAI/InferenceX-app#1495), which this checkout cannot list. The question is whether it ever sends a report that is partly filled in. Examples: an ingest that fails after counting benchmark rows but before power or trace, a non-agentic run that leaves out trace, or a detail key set to null. At ingest_results_callback.py:63-67, a missing category raises "Callback report is missing {category} counts". _count(row.get(o)) at :44-45 raises on a missing outcome (None). _count(detail[key]) at :71 raises on a null detail value. report() calls render_report at :132, before merged_pr and before any POST or PATCH. So the RuntimeError ends the run in __main__ with exit 1, and the PR gets nothing. The ❌ failure notice and app-run link are lost on exactly the failure path where they matter most. The only fallback is report is None at :54-55, which covers only a fully missing report, not a partial one.

Verification: nit. render_report raises at lines 65-66 when a category is missing, at lines 44-45 via line 67 for a missing outcome, and at line 71 for a null detail key. In report(), line 132 runs before the POST/PATCH at lines 185-199, so sys.exit(1) at lines 216-218 leaves the PR with no comment, not even the "❌ Ingest ... failed" status. Only report is None at line 54 degrades gracefully.

@Oseltamivir
Oseltamivir merged commit a9a8aa3 into main Oct 25, 2026
4 checks passed
@Oseltamivir
Oseltamivir deleted the feat/ingest-report-callback branch October 25, 2026 10:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

1 participant