Skip to content

[Feature] review-bot supports issue assignment #87

Closed

Description

@zengchuang-hw
No description provided.

Activity

  1. zengchuang-hw commented on Aug 17, 2026

    @zengchuang-hw
    ContributorAuthor

    @TaffyOfficial plz help to improve the review bot

  2. TaffyOfficial commented on Aug 17, 2026

    @TaffyOfficial
    Contributor

    Implemented and deployed: https://github.com/zuiho-kai/omni-reviewbot/pull/3

    The bot now scans newly opened Issues separately from PRs, routes model/module owners, adds real GitHub assignees once, and shows Issue assignment jobs in the dashboard. Existing open Issues were recorded without backfill.

  3. TaffyOfficial commented on Aug 17, 2026

    @TaffyOfficial
    Contributor

    Follow-up: adjusted in https://github.com/zuiho-kai/omni-reviewbot/pull/4. Issue routing now only mentions owners in a deduplicated comment and does not modify GitHub Assignees. It is currently enabled for vllm-omni and disabled for vllm-gr. This issue remains open.

  4. zengchuang-hw commented on Aug 18, 2026

    @zengchuang-hw
    ContributorAuthor

    bug issue maybe not properly assigned.
    vllm-project/vllm-omni#6258
    ci error should not assigned to wangyu but the test case owner
    @tzhouam ptal

  5. tzhouam commented on Aug 18, 2026

    @tzhouam
    Collaborator

    Traced the #6258 misroute. @zengchuang-hw is right, and the cause was structural rather than a bad classification.

    What happened. _CI_OWNER in owner_router.py was a single hardcoded handle, so every issue classified as CI work was mentioned to @yenuo26 — who, on vllm-project/vllm-omni#6258, was also the reporter, since the nightly report files under that account. He had already asked @Bounty-hunter to look at it three minutes before the bot posted. Both routing paths converged on the same handle: the agent answered kind=ci, and the ci-failure label fallback returned the same route when the agent was unavailable. The model fallback further down did resolve #6258 to @Bounty-hunter, but a ci-failure label meant it was never reached.

    @TaffyOfficial already fixed the main case in zuiho-kai/omni-reviewbot#10 — a labelled CI failure now routes to the model owner when the title names the model, before the CI classifier runs.

    One gap remains, and zuiho-kai/omni-reviewbot#9 closes it. Measured across the 36 open ci-failure issues, all of which are mentioned to @yenuo26 today:

    signal routes
    #10, model in the title 10 / 36
    #9, model in the failing test node 9 / 36
    combined 12 / 36

    Neither is a superset of the other. #10 alone catches the perf-regression reports (#6300, #6015, #5968) that name a model but run no single test. #9 alone catches #5786 and #5595, where GitHub truncates the long title mid-path and the model name survives only in the generated body. #9 is rebased on #10 and changes none of its placement, precedence or prompt wording.

    The 12 keep their wrong comment either way, because the watcher comments once per issue. #9 adds an opt-in command rather than doing it automatically:

    omni-reviewbot reroute-issues 6300 6295 6258 6247 6144 6143 6015 5968 5962 5786 5736 5595
    omni-reviewbot reroute-issues 6258 --post
    

    It takes explicit numbers, never scans, prints the corrected route unless --post is passed, skips issues whose route did not change, and never posts twice. Whether to correct the public threads is @TaffyOfficial's call.

    The bigger lever is the catalog, and it needs a human. Of the 24 issues that still route to CI, only a few genuinely belong there — #6252 is a packaging break, #5776 an entrypoint L3 failure, #6245 and #5733 quantization-harness failures whose fp8_ltx2 parameter matches no model. The rest are missing data, not a routing decision: Qwen3-Omni (5 issues), FLUX, Gepard-1.0, Dreamzero, cosyvoice3, VoxCPM2, step_audio2, moss_tts, Higgs, soulxsinger, sensenova_u1 have no entry in model_owners.json. wan22 also misses, because the aliases list wan2.2 and wan 2 2 but not the form used in test_wan22_expansion.

    Adding those owners would route more issues than either code change here. @zengchuang-hw — can you name the owners for those models, or point at someone who can? That is the last thing blocking full coverage.

    One note on scope for the record: nothing in InferMatrixCopilot assigns issues. Its agent.triage_issues step is read-only and routes to a module string, and vllm-omni's own Mergify config only assigns on pull requests. All the issue routing discussed here lives in omni-reviewbot.

  6. zengchuang-hw commented on Aug 27, 2026

    @zengchuang-hw
    ContributorAuthor

    Thanks for the detailed analysis. Please proceed with #9/#10. I’ll collect the missing model owners and update model_owners.json separately; for now, please reroute #6258 with --post.

  7. tzhouam commented on Sep 11, 2026

    @tzhouam
    Collaborator

    Two parts of this shipped; the rest needs a decision only the team can make.

    Shipped

    The wan22 alias miss (JiusiServe/omni-reviewbot#60). The catalog looked complete — the Wan entry listed wan2.1, wan2_1, wan-2.1, wan 2 1 and the 2.2 equivalents — but _model_match_text replaces punctuation with spaces before comparing, so all four spellings collapse to one token run. The form vllm-omni actually uses in test ids has no separator and normalizes to a single indivisible token:

    tests/e2e/test_wan22_expansion.py::test_x  -> 'tests e2e test wan22 expansion py test x'  -> None
    tests/e2e/test_wan2_2_expansion.py::test_x -> 'tests e2e test wan2 2 expansion py test x' -> ('bjf-frz',)
    

    Four alias spellings, one effective alias, and not the one the tests use. wan21 and wan22 are added.

    An ownerless entry no longer takes routing down silently (JiusiServe/omni-reviewbot#61). route_issue tested the route object, not its owners, so a catalog entry that matched but listed no owners returned a truthy route with nobody in it — short-circuiting every remaining signal into a no_owner outcome that named nothing. A CI-labelled issue in that state also fell through to the CI owner, which is the misroute measured here. The gap is now carried out and named instead.

    Not shipped — the catalog itself

    The measurement in this thread still stands: the routing decision is mostly fine, the data is missing. These have no entry at all:

    Qwen3-Omni, FLUX, Gepard-1.0, Dreamzero, cosyvoice3, VoxCPM2, step_audio2, moss_tts, Higgs, soulxsinger, sensenova_u1

    Adding them needs an owner per model. Commit history cannot supply that — it shows who touched a path, not who is accountable for it, and guessing would reproduce exactly the failure #139 reported (someone mentioned for a module they do not own). The catalog currently holds 9 entries, and every model outside them routes on weaker signals.

    What would close this: a name per model above, in model_owners.json and the matching owners/model_owner.md row. If some of them have no owner yet, saying so is also useful — an entry can carry that explicitly now without breaking routing.

    The other half of this issue — the 12 already-miscommented ci-failure issues keeping their wrong mention, since the watcher comments once per issue — is unchanged and still needs the opt-in re-route command to be run.

    🤖 Generated with Claude Code

    https://claude.ai/code/session_01KT4EvUTzzLeq243DG6T1mC

  8. tzhouam commented on Sep 29, 2026

    @tzhouam
    Collaborator

    Closing. The routing work shipped: CI test-node ownership signal (JiusiServe/omni-reviewbot#27), module-over-model owner scoring (#31), the compact Wan alias fix (#60) and ownerless catalog entries treated as gaps (#61); issue assignment is live in production (ISSUE_ASSIGNMENT_ENABLED=true). The remaining asks need a team decision; please open a focused issue if one is still wanted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions