Skip to content

api: resolve patchset summary by slug as fallback - #499

Open
zraza-xlnx wants to merge 1 commit into
sashiko-dev:mainfrom
zraza-xlnx:user/zraza/api-patchset-summary-by-slug
Open

zraza-xlnx wants to merge 1 commit into
sashiko-dev:mainfrom
zraza-xlnx:user/zraza/api-patchset-summary-by-slug

Conversation

@zraza-xlnx

Copy link
Copy Markdown
Contributor

The GET /api/patchset summary handler only looked up patchsets by numeric id or message-id. Forge ingested patchsets are addressed by slug (for example reponame-79), so the web UI's summary requests for those patchsets fell through to the message-id lookup, matched nothing, and returned 404.

UI links reviews by slug, but the API never resolved slugs. This PR adds that fallback so slug links stop 404 error.

const target = `#/patchset/${encodeURIComponent(p.slug || p.message_id || p.id)}`;

Chain the non numeric lookups by result: try message-id first, then fall back to slug when the message-id lookup finds nothing. This mirrors the branching already used by the patchset details handler.

Regression test results

Test area Example Parent PR
Repository-name and legacy slug lookup id=linux-next-725 (reponame-prnum) FAIL: HTTP 404 PASS: HTTP 200
Numeric/message-ID lookup id=42 selects numeric patchset 42 PASS PASS

This change is backward compatible.

@zraza-xlnx

Copy link
Copy Markdown
Contributor Author

Reviewed by @pieterj-xilinx and @alexaust-xlnx internally.

@zraza-xlnx
zraza-xlnx marked this pull request as draft September 14, 2026 11:45
@zraza-xlnx
zraza-xlnx marked this pull request as ready for review September 14, 2026 12:05
The GET /api/patchset summary handler only looked up patchsets by
numeric id or message-id. Forge ingested patchsets are addressed by
slug (for example reponame-79), so the web UI's summary requests for
those patchsets fell through to the message-id lookup, matched
nothing, and returned 404.

Chain the non numeric lookups by result: try message-id first, then
fall back to slug when the message-id lookup finds nothing. This
mirrors the branching already used by the patchset details handler.

Signed-off-by: Zuhair Hasan Raza <ZuhairHasan.Raza@amd.com>
@zraza-xlnx
zraza-xlnx force-pushed the user/zraza/api-patchset-summary-by-slug branch from ea697a9 to ec4a4db Compare September 18, 2026 09:04
@sashiko-bot

sashiko-bot Bot commented Sep 18, 2026

Copy link
Copy Markdown

Sashiko review — v2

Commit 1/1 — ec4a4db3 api: resolve patchset summary by slug as fallback

  • [HIGH] In src/api.rs (get_patchset_summary), the sequential database
    fallback omits the fast string heuristic used elsewhere. As a result,
    looking up a valid slug forces an unindexed full table scan on
    message_id, which can cause severe latency and SQLite stalls on a
    growing database. Furthermore, valid message ID lookups defer their log
    until after the query completes, and missing message IDs incorrectly fall
    back to log and search as a slug.

Full review and stage logs on sashiko.sashiko.dev

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.

1 participant