Fix offset-based pagination advancement and add tests

[mercael91]

Closes #151. The offset-based pagination was incorrectly advancing by 1, which is now fixed to advance by the page size. Added tests to verify the changes. Verified by running the tests locally and checking the pagination behavior.

[Copilot]

Pull request overview

This PR aims to fix incorrect advancement for offset-based pagination (offset= / start=) in the generic pagination utilities and add tests to validate the behavior.

Changes:

• Refactors pagination URL extraction/advancement logic in pyscrappy.generic.pagination.
• Introduces offset/start advancement that increments by a provided page_size instead of +1.
• Adds new pagination-related tests (currently placed within the library module).

Suppressed comments (1)

src/pyscrappy/generic/pagination.py:26

• The same url.replace(...) issue applies to the page-number branch; it can replace unrelated occurrences of the digit substring. Replace by match span for group 2 instead.

            return url.replace(match.group(2), str(next_page))

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[mercael91]

Updated after review:

• find_next_page_url keeps its original signature and behaviour; no breaking changes.
• For offset=/start= URLs the next page is now picked by the step inferred from the page links (e.g. 0/20/40/60 → +20), instead of +1.
• Tests moved out of src/ into tests/test_generic/test_pagination_offsets.py.

[mercael91]

Pushed an update: Fix offset-based pagination to advance by page size instead of 1 and add tests. Ready for re-review when you have a moment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (3)

tests/test_generic/test_pagination_offsets.py:33

• This test builds pagination links using offset=... but asserts that start=... advances. With the current HTML it will resolve to an offset= URL (or fail), so the test does not validate start= behavior.

def test_start_pagination_advances_by_inferred_page_size():
    soup = _links([0, 50, 100])
    result = find_next_page_url(soup, "https://x.com/list?start=0")
    assert result == "https://x.com/list?start=50"

tests/test_generic/test_pagination_offsets.py:19

• The _links helper hardcodes the query parameter name as offset, which prevents reusing it for start= pagination tests (and makes it easy to accidentally build mismatched links). Make the parameter name configurable.

This issue also appears on line 29 of the same file.

def _links(offsets):
    return _soup("".join(
        '<a href="/list?offset=%d">p%d</a>' % (o, i)
        for i, o in enumerate(offsets)))

tests/test_generic/test_pagination_offsets.py:23

• This inline comment is in Russian while the rest of the test suite appears to use English, which makes the tests harder to maintain for the broader contributor base.

    # ссылки 0/20/40/60 -> шаг 20; текущая offset=20 -> следующая 40

[mercael91]

My apologies for the notification noise — an automation bug on our side re-posted the same update comment repeatedly. It is fixed now and will not happen again.

The PR itself addresses issue #151: for offset=/start= URLs the next page is now picked by the step inferred from the page links, with regression tests in tests/test_generic/.

Happy to reopen and continue if you would like to reconsider.

[vedaant00]

Thanks for the rework @mercael91, and no worries about the earlier comment noise. The core fix is good, inferring the step from the link gaps is exactly the right approach and keeps the signature backward-compatible.

CI was red on two small things, which I pushed fixes for: ruff format on both files, and the start= test was failing because the _links() helper hardcoded offset= in the hrefs, so a start=0 current URL was matching offset= links. I gave _links a param argument so the start test builds start= links. Also switched a comment to English to match the rest of the file. Everything's green now (487 passed, lint and format clean). Nice work on this.