fix: clamp page overflow to last page in /refractor status (#141) #142

Claude · 2026-04-07T15:32:12Z

Claude commented

2026-04-07 15:32:12 +00:00

Closes #141

What changed

When a user passes a page beyond the total (e.g. /refractor status page:999), the API returns {"count": 13, "items": []}. The cog was falling through to the "No refractor data found" message without checking that count > 0.

Added a re-fetch guard in refractor_status (after the initial API call): if items is empty but total_count > 0, the page is clamped to ceil(total_count / PAGE_SIZE) and a second request is made with the corrected offset. This mirrors the clamping already done by the pagination buttons in RefractorPaginationView._fetch_and_update.

Files changed

cogs/refractor.py — 12-line insertion after total_count is computed

Test coverage

REF-22 in tests/refractor-integration-test-plan.md describes this exact scenario. No automated test exists for this path; manual verification against team 31 on dev (13 cards) is recommended.

Closes #141 ## What changed When a user passes a page beyond the total (e.g. `/refractor status page:999`), the API returns `{"count": 13, "items": []}`. The cog was falling through to the "No refractor data found" message without checking that `count > 0`. Added a re-fetch guard in `refractor_status` (after the initial API call): if `items` is empty but `total_count > 0`, the page is clamped to `ceil(total_count / PAGE_SIZE)` and a second request is made with the corrected offset. This mirrors the clamping already done by the pagination buttons in `RefractorPaginationView._fetch_and_update`. ## Files changed - `cogs/refractor.py` — 12-line insertion after `total_count` is computed ## Test coverage REF-22 in `tests/refractor-integration-test-plan.md` describes this exact scenario. No automated test exists for this path; manual verification against team 31 on dev (13 cards) is recommended.

Claude added 1 commit 2026-04-07 15:32:13 +00:00

fix: clamp page overflow to last page in /refractor status (#141 )

Ruff Lint / lint (pull_request) Successful in 30s

Details

cb17b99220

When page exceeds total pages, API returns empty items but non-zero
count. Now detects this case and re-fetches the last valid page.

Closes #141

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Claude referenced this pull request

2026-04-07 15:32:23 +00:00

bug: /refractor status page overflow shows "no data" instead of clamping to last page #141

Claude added the

ai-reviewing

label 2026-04-07 15:45:58 +00:00

Claude reviewed 2026-04-07 15:47:28 +00:00

Claude left a comment

AI Code Review

Files Reviewed

cogs/refractor.py (modified — 12-line insertion)

Findings

Correctness

No issues found.

The guard (if not items and total_count > 0) fires exactly when the API returns {"count": N, "items": []} due to an out-of-range offset. The ceiling-division formula (total_count + PAGE_SIZE - 1) // PAGE_SIZE is correct. page is set to total_pages before building clamped_params, so (page - 1) * PAGE_SIZE computes the right offset for the last page.

Two things I checked specifically:

params passed to RefractorPaginationView is stale (still has the original out-of-range offset) — not a bug. _fetch_and_update strips "offset" from base_params and recomputes it from self.page, so the view always fetches the correct page regardless.
page and total_pages are recomputed again at lines 400–401 after the guard — the recomputation uses the updated total_count from the re-fetch; since the total hasn't changed, both values remain correct and the subsequent page = min(page, total_pages) is a no-op clamp at the already-correct last-page value.

Security

No issues found. All params are from validated app_commands choices or computed from them.

Style & Conventions

No issues found. Filter-and-reappend pattern for params ([(k, v) for k, v in params if k != "offset"] + append) is consistent with _fetch_and_update. Comment is clear and scoped.

Suggestions

No automated test for this path. REF-22 in the integration plan documents the scenario; the PR recommends manual verification against team 31 on dev. A unit test mocking db_get to return {"count": 13, "items": []} on the first call and a populated list on the second would lock this in permanently — optional, doesn't block merge.

Verdict: APPROVED

Correct fix for the described edge case. Logic is sound, no security or convention issues. The interaction with the stale params passed to RefractorPaginationView is safe because the view always recomputes its own offset. Gitea blocks self-approval — posting as COMMENT.

Automated review by Claude PR Reviewer

## AI Code Review ### Files Reviewed - `cogs/refractor.py` (modified — 12-line insertion) ### Findings #### Correctness No issues found. The guard (`if not items and total_count > 0`) fires exactly when the API returns `{"count": N, "items": []}` due to an out-of-range offset. The ceiling-division formula `(total_count + PAGE_SIZE - 1) // PAGE_SIZE` is correct. `page` is set to `total_pages` before building `clamped_params`, so `(page - 1) * PAGE_SIZE` computes the right offset for the last page. Two things I checked specifically: - **`params` passed to `RefractorPaginationView` is stale (still has the original out-of-range offset)** — not a bug. `_fetch_and_update` strips `"offset"` from `base_params` and recomputes it from `self.page`, so the view always fetches the correct page regardless. - **`page` and `total_pages` are recomputed again at lines 400–401** after the guard — the recomputation uses the updated `total_count` from the re-fetch; since the total hasn't changed, both values remain correct and the subsequent `page = min(page, total_pages)` is a no-op clamp at the already-correct last-page value. #### Security No issues found. All params are from validated `app_commands` choices or computed from them. #### Style & Conventions No issues found. Filter-and-reappend pattern for `params` (`[(k, v) for k, v in params if k != "offset"]` + `append`) is consistent with `_fetch_and_update`. Comment is clear and scoped. #### Suggestions - No automated test for this path. REF-22 in the integration plan documents the scenario; the PR recommends manual verification against team 31 on dev. A unit test mocking `db_get` to return `{"count": 13, "items": []}` on the first call and a populated list on the second would lock this in permanently — optional, doesn't block merge. ### Verdict: APPROVED Correct fix for the described edge case. Logic is sound, no security or convention issues. The interaction with the stale `params` passed to `RefractorPaginationView` is safe because the view always recomputes its own offset. Gitea blocks self-approval — posting as COMMENT. --- *Automated review by Claude PR Reviewer*

Claude added

ai-reviewed

and removed

ai-reviewing

labels 2026-04-07 15:47:54 +00:00

cal merged commit 8f9242bed8 into main

2026-04-07 15:52:07 +00:00

cal deleted branch issue/141-bug-refractor-status-page-overflow-shows-no-data-i

2026-04-07 15:52:08 +00:00

cal referenced this issue from a commit

2026-04-07 15:52:10 +00:00

Merge pull request 'fix: clamp page overflow to last page in /refractor status (#141)' (#142) from issue/141-bug-refractor-status-page-overflow-shows-no-data-i into main