Repository navigation
feat(ingest): comment app ingest reports on the merged PR / ingest:在已合并的 PR 上评论 app 的 ingest 报告 - #4503
Conversation
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 的统计;重跑或失败时原地更新。
|
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 包的改动有更高的代码质量要求。 |
| 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)} |") |
There was a problem hiding this comment.
🟡 (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.
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
ingest-results-callback.yml(repository_dispatch: ingest-results-completed) runsinfx.workflows.ingest_results_callback, mirroringstage-results-callback.yml.merge-run-id, requires it to be amerge-ingest.ymlpush tomain, and picks the PR whosemerge_commit_shais that run'shead_sha(/commits/{sha}/pulls), i.e. the PR whose changelog entry was published.<!-- ingest-results-report merge-run=<id> -->comment and PATCHes it, so reruns and failure-then-recovery update in place.Why a callback: the app's
INFX_MAIN_PATonly reads artifacts, and the app already usesPATto dispatch staging callbacks here, which then comment withgithub.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。