langgenius / langgenius/dify

Annotation hit-history pager never ends for limit=0: has_more is computed from a page size the query does not use

Open
#42,322 1 comment 1 reaction 0 assignees View on GitHub
Dominant language
TypeScript
Stars
156k
Forks
24.6k
Avg merge
22h 9m
Merged PRs (30d)
610

Description

### Self Checks

- [x] I have searched for existing issues, and this has not been reported before
- [x] I am using the latest `main`

### Dify version

`main` at `a4d74f7c`

### Cloud or Self Hosted

Self Hosted (Source)

### Steps to reproduce

`GET /console/api/apps/{app_id}/annotations/{annotation_id}/hit-histories?limit=0`

That route reads both pagination parameters straight off the query string with no bounds:

```python
page = request.args.get("page", default=1, type=int)
limit = request.args.get("limit", default=20, type=int)
effective_limit = min(limit, 100)
```

`min(limit, 100)` has no lower bound, but `libs/pagination.paginate_query` floors both at 1 before it runs the query:

```python
page = max(1, page)
per_page = max(1, per_page)
```

So the query is served with a page size of 1 while the response reports `limit: 0`, and `has_more` is computed from the requested value:

```python
has_more=page * effective_limit < total
```

`page * 0 < total` is true for every page, so a client that walks pages until `has_more` is false never stops. Past the end it keeps receiving empty pages that still claim there is more. `limit=-1` behaves the same way.

Driving the real `paginate_query` against a seven-row table, with the handler's own arithmetic:

```
--- client asks limit=0, walks pages until has_more is false ---
page=1 rows=1 ids=[1] has_more=True (paginate_query.has_next=True)
...
page=7 rows=1 ids=[7] has_more=True (paginate_query.has_next=False)
page=8 rows=0 ids=[] has_more=True (paginate_query.has_next=False)
page=9 rows=0 ids=[] has_more=True
page=10 rows=0 ids=[] has_more=True
-> client never stopped after 10 pages

--- the same walk with limit=3 ---
page=3 rows=1 ids=[7] has_more=False -> client stops here
```

`PaginatedResult.has_next` is right in every one of those rows; it is the handler's separate recomputation from unclamped inputs that is not.

`page` has the same gap on this route: `?page=0` is served as page 1 while the response echoes `page: 0`.

The annotation list routes (`AnnotationApi.get`, service API `AnnotationListApi.get`) carry the same recomputation, introduced together with it in #41876. Those are currently protected because their query models declare `limit: int = Field(default=20, ge=1)`, so only the hit-history route is reachable today.

### ✔️ Expected Behavior

`has_more` is false once the last page has been served, and `limit`/`page` in the response are the values the query actually used.

### ❌ Actual Behavior

`has_more` stays true forever for `limit=0` or `limit=-1`, and the response reports a page size that was not used.

Contributor guide

Open the contributing guide

Research direction

Start at the hit-history route for GET /console/api/apps/{app_id}/annotations/{annotation_id}/hit-histories and inspect how it calls libs/pagination.paginate_query. Compare the handler's page and limit values with the values actually used by pagination, then check the related AnnotationApi.get and AnnotationListApi.get recomputations. Done means invalid bounds cannot produce endless has_more responses and the returned page and limit match the query.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.