Publish crate pagination docs, tests, and exec plan (4.1.3)

[leynos]

Summary

• Expand crate-level pagination documentation (ordering requirements, default/max limits, error mapping, and scope boundaries).
• Improve module-level documentation for cursor, envelope, and params modules to reflect integration details and edge-cases.
• Add a new ExecPlan document describing the plan for 4.1.3 (docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md).
• Update architecture and roadmap docs to reflect completion of item 4.1.3.
• Public API surface remains unchanged; changes are documentation and tests only.
• Add shared test fixtures and BDD driver support for documentation invariants in tests/common.rs.
• Update docs/developers-guide.md to describe shared workspace crate testing layout and guidance.

Changes

Documentation

• Expand crate-level docs in backend/crates/pagination/src/lib.rs with four sections:
  • Ordering requirements
  • Default and maximum limits (documenting DEFAULT_LIMIT = 20 and MAX_LIMIT = 100)
  • Error mapping guidelines for CursorError and PageParamsError
  • Scope boundaries (what the crate does not provide)
• Improve module-level documentation:
  • cursor.rs gains notes on base64url encoding, direction behavior, and cursor opacity, plus security considerations about opacity (not signed/encrypted).
  • envelope.rs notes behavior of from_request and optional links serialization, including skip_serializing_if behavior for nulls.
  • params.rs notes on Deserialize normalization and framework integration.
• New ExecPlan document: docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md describing the execution plan, scope, risks, and acceptance criteria.

Tests

• Add unit tests (using rstest) to verify documented invariants in:
  • params.rs (consistency of DEFAULT_LIMIT/MAX_LIMIT with normalization)
  • cursor.rs (stable error display strings for CursorError variants)
• Add behavioural tests (using rstest-bdd):
  • New BDD feature file: backend/crates/pagination/tests/features/pagination_documentation.feature
  • Extend backend/crates/pagination/tests/pagination_bdd.rs with steps for the new scenarios
  • Scenarios cover default limit, max limit, zero limit error, and error variant mappings
• New test artifacts
  • BDD feature: backend/crates/pagination/tests/features/pagination_documentation.feature
  • Updated BDD driver to include documentation invariants
  • ExecPlan document: docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md

New/Updated test scaffolding

• Add shared test fixtures and world state for BDD in backend/crates/pagination/tests/common.rs.
• Add dedicated documentation invariant tests: backend/crates/pagination/tests/pagination_documentation_bdd.rs and backend/crates/pagination/tests/pagination_documentation.feature.
• Update docs/developers-guide.md to describe shared workspace crate testing layout and guidance.

Documentation/testing gates + architecture/roadmap updates

• Ensure cargo doc -p pagination --no-deps builds without warnings
• Ensure crate tests pass: cargo test -p pagination (with new tests)
• Lint/fmt gates: make check-fmt, make lint, make test must pass
• Update references to reflect completion:
  • docs/wildside-backend-architecture.md updated with the pagination documentation scope decision
  • docs/backend-roadmap.md updated to mark 4.1.3 as done

Validation / Test Plan

• Unit tests added for documentation invariants (rstest) and pass with cargo test -p pagination.
• Behavioural tests added (rstest-bdd) and pass with cargo test -p pagination --test pagination_bdd.
• Documentation build succeeds: cargo doc -p pagination --no-deps with no warnings.
• No public API changes; API surface remains the same.
• Formatting and lint gates pass: make check-fmt, make lint, make test.
• Architecture notes and roadmap updated to reflect completion of item 4.1.3.

Rationale

• This update provides a self-contained reference for developers using the pagination primitives and invariant verification via unit and BDD tests. It also aligns repository documentation with the updated architecture/roadmap.

How to review

• Focus on the new crate-level documentation sections in lib.rs and the expanded module docs in cursor.rs, envelope.rs, and params.rs.
• Review new tests for coverage of documented invariants and ensure they align with the documented behavior.
• Verify the new BDD feature and step definitions reflect the documented scenarios.
• Check that docs/wildside-backend-architecture.md and docs/backend-roadmap.md accurately record the completion of 4.1.3.

Notes

• No production dependencies were added; this is a documentation and test enhancement.
• Edits are additive and do not modify the existing API or behavior beyond what the docs describe.

📎 Task: https://www.devboxer.com/task/f1f2e744-e11b-4ed1-93df-797a877e8dcd

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: e5be07c7-d3e7-4f24-acc8-eb167816b79c

📥 Commits

Reviewing files that changed from the base of the PR and between d264000 and ccb0832.

📒 Files selected for processing (2)

• backend/src/inbound/http/users/tests.rs
• backend/src/inbound/http/users/tests/request_validation_tests.rs

---

Publish crate-level pagination documentation and tests (roadmap 4.1.3)

This PR adds comprehensive Rustdoc, tests, BDD fixtures, developer guidance, and a living ExecPlan for the pagination crate without changing its public API or adding production dependencies. See docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md for the execution plan, scope, risks, quality gates and acceptance evidence.

Documentation

• Crate-level (backend/crates/pagination/src/lib.rs)
  • Ordering requirements for Cursor key types (serialization bounds, total ordering aligned with composite DB indices and UUID tie-breakers) and explicit note that runtime/property-based ordering checks are the consumer's responsibility.
  • Page-size normalisation: DEFAULT_LIMIT = 20, MAX_LIMIT = 100; None → default, over-limit → clamp to MAX_LIMIT, zero → PageParamsError::InvalidLimit.
  • Error-mapping guidance recommending HTTP status and envelope code mappings for CursorError and PageParamsError (e.g., CursorError::Serialize → HTTP 500).
  • Clear scope boundaries (what the crate does not provide: Diesel filter builders, Actix extractors, connection pooling, OpenAPI generation).
• Module docs
  • cursor.rs: base64url-encoded JSON cursor format, dir semantics (default Next), and opacity/security note (opaque but not signed/encrypted).
  • envelope.rs: Paginated/PaginationLinks semantics; PaginationLinks::from_request preserves non-pagination query parameters; next/prev are omitted when None.
  • params.rs: Deserialize normalisation behaviour and notes on framework/query-extractor integration.
• Architecture & guides
  • docs/wildside-backend-architecture.md: added 4.1.3 section describing ordering semantics, constants and HTTP mapping.
  • docs/developers-guide.md: shared-workspace BDD guidance, common.rs pattern, Direction in fixture examples, and a quality-gate checklist.
  • docs/backend-roadmap.md: marks 4.1.3 complete.
• ExecPlan
  • Added living ExecPlan document with scope, constraints, risks, mitigations, owners, quality gates and completion evidence (docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md).

Tests & BDD

• BDD
  • New feature: backend/crates/pagination/tests/features/pagination_documentation.feature covering documented invariants (default limit normalisation, clamping to MAX_LIMIT, zero rejection, cursor-decoding error categories and human-readable error strings).
  • New/updated scenario bindings: backend/crates/pagination/tests/pagination_documentation_bdd.rs and updated pagination_bdd.rs.
  • Shared fixtures/helpers consolidated into backend/crates/pagination/tests/common.rs (World, FixtureKey, re-exports, set_page_params_with_limit).
  • Removed dangling no-op step and refactored duplicated cursor extraction into decoded_cursor(world: &World) helper.
• Unit tests
  • params.rs: boundary tests (MAX_LIMIT ±1) and parameterised clamping tests.
  • cursor.rs: tests for CursorError display strings and a CursorError::Serialize path via a key type that fails serialization; cursor-decoding-errors scenario strengthened to assert exact error counts.
• Test scaffolding
  • BDD driver and step implementations updated to use shared fixtures and helpers.

Wider test and infra changes (non-production)

• Broad migration across tests to fallible Result/TestResult-based helpers instead of panic-on-failure (.expect), mapping mutex-poison/lock failures to domain errors and propagating errors via ? for robustness (changes across backend/src/* and many inbound/outbound test modules).
• Example-data generator reworked: removed old generator.rs and added a deterministic generator module with comprehensive tests (crates/example-data/src/generator/mod.rs and tests) for reproducible UUIDs, display-name generation, bounded subset selection and unit-system distribution.
• Numerous test cleanups: parameterisations, deterministic replacements for probabilistic tests, helper extractions and small doc-example fixes.

Other

• scripts/check-overrides-parity.test.mjs updated to include uuid@14.0.0.
• New catalogue theme tests (backend/src/domain/catalogue/tests/theme_tests.rs) and several doc-example import fixes.

Quality gates and acceptance

• Pagination crate validation gates documented and satisfied: cargo doc -p pagination --no-deps (no warnings), cargo test -p pagination (passes), repo-level quality gates (make check-fmt, make lint, make test).
• Changes are additive (docs/tests/helpers); public API surface unchanged and no production dependencies added.

Walkthrough

Summarise expanded pagination Rustdoc and BDD/tests: document cursor encoding (base64url JSON), ordering and dir semantics, limit normalisation and error→HTTP mappings; add shared pagination BDD fixtures and tests; replace example-data generator with a deterministic implementation and add tests; convert many test suites to fallible Result flows. No public runtime APIs changed.

Changes

Cohort / File(s)	Summary
Pagination crate docs & module comments backend/crates/pagination/src/lib.rs, backend/crates/pagination/src/cursor.rs, backend/crates/pagination/src/envelope.rs, backend/crates/pagination/src/params.rs	Expand crate/module Rustdoc: specify cursor encoding as base64url(JSON), document optional dir default Next, state ordering invariants and cross-page stability, and describe DEFAULT/MAX limit normalisation and error→HTTP/status mapping. No API or runtime changes.
Pagination tests & BDD fixtures backend/crates/pagination/tests/common.rs, backend/crates/pagination/tests/pagination_bdd.rs, backend/crates/pagination/tests/pagination_documentation_bdd.rs, backend/crates/pagination/tests/features/*	Add shared BDD support (common.rs) with re-exports and World state; refactor step bindings to use helpers; add feature asserting documentation invariants and cursor decode-failure tests.
Docs, roadmap & execplan docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md, docs/wildside-backend-architecture.md, docs/backend-roadmap.md, docs/developers-guide.md	Add ExecPlan and architecture/developer guidance describing pagination semantics, testing conventions and scope boundaries; mark roadmap item 4.1.3 complete; provide guidance for shared BDD fixtures and documentation-invariant tests.
Example-data generator replacement crates/example-data/src/generator.rs (removed), crates/example-data/src/generator/mod.rs (added)	Remove old generator; add deterministic generator using ChaCha8Rng seeded from provided seed, RNG-derived UUIDs, sanitised display-name retry with explicit failure error, bounded deterministic subset selection and unit-system selection.
Example-data generator tests crates/example-data/src/generator/tests.rs	Add deterministic tests covering counts, repeatability, seed divergence, display-name validation and retry failure, bounded subset selection, and unit-system selection behaviour.
Seed registry CLI tests refactor crates/example-data/src/seed_registry_cli/tests.rs	Refactor fixtures/tests to fallible Result flows; propagate errors with ?; make cleanup best-effort rather than panicking.
Widespread test hardening backend/src/... (many files; e.g., backend/src/domain/..., backend/src/inbound/..., backend/src/outbound/..., backend/src/middleware/...)	Convert many test helpers and tests from panic/expect(...) to fallible TestResult (Result<..., Box<dyn Error>>), propagate errors with ?, map mutex-poison/absent values to domain/io errors, and ensure tests return Ok(()).
JS parity test update scripts/check-overrides-parity.test.mjs	Add uuid@14.0.0 to synced overrides and update fixtures/assertions and expected success log to include uuid.
Small doc/example tweaks backend/src/domain/ports/..., backend/src/inbound/http/..., backend/src/domain/user_interests.rs, backend/src/inbound/http/walk_sessions.rs, ...	Adjust Rustdoc examples and imports so traits/types are in scope; simplify examples and ensure examples compile in context. No runtime behaviour changes.

Possibly related issues

• Implement pagination link generation and backwards navigation improvements #207 — Matches the pagination cursor encoding and PaginationLinks behaviour documented and tested here (base64url JSON cursors, query-parameter-preserving link generation, prev/next detection).

Suggested labels

Roadmap

Poem

Cursors tucked in base64url, JSON folded neat,
Limits capped and documented, invariants complete,
Fixtures shared and tests now fallible and clear,
Generator seeded, determinism cheer ✨

🚥 Pre-merge checks | ✅ 9

✅ Passed checks (9 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly references roadmap item 4.1.3 and accurately summarises the primary changes: publishing pagination crate documentation, tests, and the execution plan.
Description check	✅ Passed	The description comprehensively relates to the changeset, detailing documentation expansions, test additions, test scaffolding updates, and roadmap completion without being off-topic.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Testing	✅ Passed	All new functionality is properly guarded by unit tests and behavioural tests validating pagination, generator refactoring, and documentation claims.
User-Facing Documentation	✅ Passed	This pull request introduces no new user-facing functionality or behaviour. The pagination crate is internal backend infrastructure used by handlers and repositories; it is not exposed to end users. All changes are additive documentation and tests for developers and architects, including crate-level Rustdoc, module documentation, BDD test scenarios, and developer-guide guidance.
Developer Documentation	✅ Passed	The pull request fully satisfies custom check requirements with roadmap item 4.1.3 checked, execplan present and marked complete, developer guide expanded with shared workspace crate testing patterns, and architecture documentation updated with pagination crate design decisions.
Module-Level Documentation	✅ Passed	All modules in the pagination crate and test infrastructure contain appropriate module-level docstrings. Source modules and new test modules include //! documentation; public functions and structs are fully documented with /// comments.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch publish-pagination-docs-4-1-3-ebd13r

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[sourcery-ai]

Reviewer's Guide

Adds detailed crate-level and module-level pagination documentation plus tests (unit and BDD) to assert the documented invariants, along with roadmap/architecture updates and an execution plan document for roadmap item 4.1.3, without changing the public API.

Updated class diagram for pagination crate core types

classDiagram
    class Cursor~Key~ {
        +Key key
        +Direction direction
        +new(key Key) Cursor~Key~
        +with_direction(key Key, direction Direction) Cursor~Key~
        +key() Key
        +direction() Direction
        +into_inner() Key
        +into_parts() Key
        +encode() Result~String, CursorError~
        +decode(value String) Result~Cursor~Key~, CursorError~
    }

    class Direction {
        <<enumeration>>
        Next
        Prev
    }

    class PageParams {
        -Option~String~ raw_cursor
        -Option~usize~ raw_limit
        -Option~Cursor~String~~ parsed_cursor
        -usize normalised_limit
        +new(cursor Option~String~, limit Option~usize~) Result~PageParams, PageParamsError~
        +cursor() Option~String~
        +limit() usize
    }

    class PageParamsError {
        <<enumeration>>
        InvalidLimit
    }

    class Paginated~T~ {
        +Vec~T~ data
        +usize limit
        +PaginationLinks links
        +new(data Vec~T~, limit usize, links PaginationLinks) Paginated~T~
    }

    class PaginationLinks {
        +String self_
        +Option~String~ next
        +Option~String~ prev
        +new(self_ String, next Option~String~, prev Option~String~) PaginationLinks
        +from_request(url String, params PageParams, next_cursor Option~Cursor~String~~, prev_cursor Option~Cursor~String~~) PaginationLinks
    }

    class CursorError {
        <<enumeration>>
        Serialize
        InvalidBase64
        Deserialize
        +message String
    }

    class Limits {
        <<constant>>
        +DEFAULT_LIMIT : usize = 20
        +MAX_LIMIT : usize = 100
    }

    Cursor --> Direction : uses
    PageParams --> Limits : normalises_with
    PageParams --> Cursor : parses_optional
    Paginated --> PaginationLinks : has
    Paginated --> "*" T : contains
    Cursor --> CursorError : encode_decode_can_fail
    PageParams --> PageParamsError : construct_can_fail

Loading

File-Level Changes

Change	Details	Files
Expand crate-level pagination documentation with explicit ordering, limits, error-mapping, and scope guidance.	Add four new sections to lib.rs docs: ordering requirements, default/maximum limits, error mapping guidelines, and scope boundaries. Document DEFAULT_LIMIT=20 and MAX_LIMIT=100 and how PageParams normalises limits, including examples. Describe CursorError and PageParamsError variants, recommended HTTP status mappings, and envelope codes, clarifying that Serialize is a server error. Clarify what the pagination crate intentionally does not provide (e.g., Diesel filters, Actix extractors, connection pooling, OpenAPI schemas).	backend/crates/pagination/src/lib.rs
Tighten documentation invariants around page size constants and normalization via unit tests.	Add tests asserting DEFAULT_LIMIT equals 20 and MAX_LIMIT equals 100. Add test asserting PageParams::new applies DEFAULT_LIMIT when limit is None and clamps values above MAX_LIMIT to MAX_LIMIT.	backend/crates/pagination/src/params.rs
Introduce BDD feature and steps to verify documented behaviour for limits and cursor error variants.	Add pagination_documentation.feature with scenarios for default limit, max-limit capping, zero-limit error, and cursor error mappings. Extend World state with page_params_result and cursor_errors slots to support new scenarios. Add step definitions that construct PageParams with various limits and assert limits or PageParamsError::InvalidLimit as appropriate. Add steps that decode invalid cursor tokens to assert specific CursorError variants (InvalidBase64, Deserialize) and ensure error display strings are non-empty. Register a new rstest-bdd scenario function to run pagination_documentation.feature.	backend/crates/pagination/tests/features/pagination_documentation.feature backend/crates/pagination/tests/pagination_bdd.rs
Update architecture and roadmap docs plus add an ExecPlan detailing execution of roadmap item 4.1.3.	Document that crate-level pagination docs now cover ordering, limits, error mapping, and scope boundaries in the architecture doc. Mark roadmap item 4.1.3 as complete in the backend roadmap. Add a detailed execution plan for 4.1.3 describing purpose, constraints, risks, plan of work, progress, and validation steps.	docs/wildside-backend-architecture.md docs/backend-roadmap.md docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[leynos]

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Overall Comments
- The `#[when("the parameters are normalized")]` step currently does nothing and relies on normalization having already happened in the `Given` step; consider either invoking the normalization logic there or renaming/removing the step to avoid misleading readers of the feature file.
- In `cursor_decoding_errors_of_different_variants`, you only assert that each collected error has a non-empty display string; it would be safer to also assert that at least one (or the expected number of) errors were collected so this scenario fails if decoding ever stops producing errors for those inputs.

## Individual Comments

### Comment 1
<location path="docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md" line_range="370-379" />
<code_context>
+- `PageParams` — normalised pagination parameters. Constructor:
</code_context>
<issue_to_address>
**suggestion (typo):** Align spelling of “normalised/normalization” with the en-GB spelling requirement.

This document and the related architecture note mix “normalised” and “normalization” (e.g. “automatic limit normalization via the Deserialize implementation”). Please standardise on the en-GB form (e.g. “normalisation”, “normalised”) across these references to match the stated spelling constraint.

Suggested implementation:

```
- `PageParams` — normalised pagination parameters. Constructor:
  `new(cursor, limit)`. Methods: `cursor()`, `limit()`. Implements
  `Deserialize` with automatic normalisation.

```

The comment references an architecture note and possibly other documents that also mix “normalised” and “normalization”. You should search the rest of this repo (especially the related architecture note) for:
- “normalization” → replace with “normalisation”
- “normalized” → replace with “normalised”
to fully enforce the en-GB spelling requirement across all related documentation.
</issue_to_address>

### Comment 2
<location path="backend/crates/pagination/src/lib.rs" line_range="86" />
<code_context>
+//! - [`DEFAULT_LIMIT`]: 20 records per page (applied when no limit is provided)
+//! - [`MAX_LIMIT`]: 100 records per page (upper bound for all requests)
+//!
+//! The [`PageParams`] type automatically normalises limit values during
+//! construction and deserialization:
+//!
</code_context>
<issue_to_address>
**issue (review_instructions):** "normalises" should use -ize spelling to match the en-GB-oxendict (-ize) requirement.

The documentation guidelines specify en-GB-oxendict spelling, which uses -ize rather than -ise. Here, "normalises" should be updated to "normalizes" to comply.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.rs`

**Instructions:**
Comments must use en-GB-oxendict (-ize / -yse / -our) spelling and grammar.

</details>
</issue_to_address>

### Comment 3
<location path="backend/crates/pagination/src/lib.rs" line_range="128" />
<code_context>
+//! | [`PageParamsError`] | `InvalidLimit`   | 400 Bad Request | `invalid_page_params`   |
+//!
+//! Note that `CursorError::Serialize` maps to HTTP 500 (not 400) because it
+//! indicates a bug in the server (the key type could not be serialised), not a
+//! client error. Consumers should log serialization failures and investigate the
+//! root cause.
</code_context>
<issue_to_address>
**issue (review_instructions):** "serialised" should use -ize spelling per the en-GB-oxendict requirement.

To match the specified en-GB-oxendict style (which prefers -ize spellings), "serialised" should be changed to "serialized" here.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.rs`

**Instructions:**
Comments must use en-GB-oxendict (-ize / -yse / -our) spelling and grammar.

</details>
</issue_to_address>

[leynos]

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix. Include the file and symbol names indicated in the issue at the head of your response. Ensure that this is validated against the current version of the codegraph.

If further refinement to address this finding would be deleterious, please supply a clear explanatory one to two paragraph markdown message I can paste into the CodeScene web ui's diagnostic suppression function so this diagnostic can be silenced.

backend/crates/pagination/tests/pagination_bdd.rs

Comment on file

//! Behavioural tests for the pagination crate foundation.

use pagination::{Cursor, CursorError, Direction, PageParams, Paginated, PaginationLinks};
mod common;

❌ New issue: Code Duplication
The module contains 3 functions with similar structure: the_decoded_cursor_has_direction_next,the_decoded_cursor_has_direction_prev,the_decoded_cursor_key_matches_the_original_key

[coderabbitai]

♻️ Duplicate comments (1)

backend/crates/pagination/tests/pagination_bdd.rs (1)

154-163: 🧹 Nitpick | 🔵 Trivial

Extract a shared decoded-cursor helper.

Remove duplicated decode-result extraction from these step definitions. Centralize it in one private helper and reuse it to keep failure messages and setup logic synchronized.

♻️ Proposed refactor

+#[expect(
+    clippy::expect_used,
+    reason = "BDD helpers use expect for clear failures"
+)]
+fn decoded_cursor<'a>(world: &'a World) -> &'a Cursor<FixtureKey> {
+    let decode_result = world
+        .decode_result
+        .get()
+        .expect("decode result should be set");
+    decode_result
+        .as_ref()
+        .expect("cursor decoding should succeed")
+}
+
 #[then("the decoded cursor key matches the original key")]
 #[expect(
     clippy::expect_used,
     reason = "BDD steps use expect for clear failures"
 )]
 fn the_decoded_cursor_key_matches_the_original_key(world: &World) {
     let key = world.key.get().expect("key should be set");
-    let decode_result = world
-        .decode_result
-        .get()
-        .expect("decode result should be set");
-    let cursor = decode_result
-        .as_ref()
-        .expect("cursor decoding should succeed");
+    let cursor = decoded_cursor(world);
 
     assert_eq!(cursor.key(), &key);
 }
@@
 fn the_decoded_cursor_has_direction_next(world: &World) {
-    let decode_result = world
-        .decode_result
-        .get()
-        .expect("decode result should be set");
-    let cursor = decode_result
-        .as_ref()
-        .expect("cursor decoding should succeed");
+    let cursor = decoded_cursor(world);
 
     assert_eq!(cursor.direction(), Direction::Next);
 }
@@
 fn the_decoded_cursor_has_direction_prev(world: &World) {
-    let decode_result = world
-        .decode_result
-        .get()
-        .expect("decode result should be set");
-    let cursor = decode_result
-        .as_ref()
-        .expect("cursor decoding should succeed");
+    let cursor = decoded_cursor(world);
 
     assert_eq!(cursor.direction(), Direction::Prev);
 }

As per coding guidelines, "Use rstest fixtures for shared setup and to avoid repetition between tests."

Also applies to: 306-333

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/crates/pagination/tests/pagination_bdd.rs` around lines 154 - 163,
The step definitions repeatedly extract and unwrap the decoded cursor from World
(e.g., in the function the_decoded_cursor_key_matches_the_original_key using
world.decode_result and world.key); extract that logic into a single private
helper (e.g., fn get_decoded_cursor(world: &World) -> &DecodedCursor) that
performs the expect() calls and returns the decoded cursor, then replace the
duplicated unwrap blocks in the step functions (including the other occurrences
around lines 306-333) with calls to this helper so failure messages and setup
are centralized and consistent.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@backend/crates/pagination/tests/pagination_bdd.rs`:
- Around line 154-163: The step definitions repeatedly extract and unwrap the
decoded cursor from World (e.g., in the function
the_decoded_cursor_key_matches_the_original_key using world.decode_result and
world.key); extract that logic into a single private helper (e.g., fn
get_decoded_cursor(world: &World) -> &DecodedCursor) that performs the expect()
calls and returns the decoded cursor, then replace the duplicated unwrap blocks
in the step functions (including the other occurrences around lines 306-333)
with calls to this helper so failure messages and setup are centralized and
consistent.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: db8c45ae-06e9-4aa6-81ba-5ce8061a82c7

📥 Commits

Reviewing files that changed from the base of the PR and between b6cd5b9 and b3b1090.

📒 Files selected for processing (3)

• backend/crates/pagination/tests/features/pagination.feature
• backend/crates/pagination/tests/pagination_bdd.rs
• backend/crates/pagination/tests/pagination_documentation_bdd.rs

💤 Files with no reviewable changes (1)

• backend/crates/pagination/tests/features/pagination.feature

[leynos]

@coderabbitai Have the following now been resolved?

Annul any requirements that violate the en-GB-oxendict spelling (-ize / -yse / -our) conventions (for example a request to replace "normalize" with "normalise" or "artefact" with "artifact"), or where the requirement unnecessarily increases cyclomatic complexity.

Please address the comments from this code review:

## Overall Comments
- The `#[when("the parameters are normalized")]` step currently does nothing and relies on normalization having already happened in the `Given` step; consider either invoking the normalization logic there or renaming/removing the step to avoid misleading readers of the feature file.
- In `cursor_decoding_errors_of_different_variants`, you only assert that each collected error has a non-empty display string; it would be safer to also assert that at least one (or the expected number of) errors were collected so this scenario fails if decoding ever stops producing errors for those inputs.

## Individual Comments

### Comment 1
<location path="docs/execplans/backend-4-1-3-publish-crate-level-pagination-documentation.md" line_range="370-379" />
<code_context>
+- `PageParams` — normalised pagination parameters. Constructor:
</code_context>
<issue_to_address>
**suggestion (typo):** Align spelling of “normalised/normalization” with the en-GB spelling requirement.

This document and the related architecture note mix “normalised” and “normalization” (e.g. “automatic limit normalization via the Deserialize implementation”). Please standardise on the en-GB form (e.g. “normalisation”, “normalised”) across these references to match the stated spelling constraint.

Suggested implementation:

```
- `PageParams` — normalised pagination parameters. Constructor:
  `new(cursor, limit)`. Methods: `cursor()`, `limit()`. Implements
  `Deserialize` with automatic normalisation.

```

The comment references an architecture note and possibly other documents that also mix “normalised” and “normalization”. You should search the rest of this repo (especially the related architecture note) for:
- “normalization” → replace with “normalisation”
- “normalized” → replace with “normalised”
to fully enforce the en-GB spelling requirement across all related documentation.
</issue_to_address>

### Comment 2
<location path="backend/crates/pagination/src/lib.rs" line_range="86" />
<code_context>
+//! - [`DEFAULT_LIMIT`]: 20 records per page (applied when no limit is provided)
+//! - [`MAX_LIMIT`]: 100 records per page (upper bound for all requests)
+//!
+//! The [`PageParams`] type automatically normalises limit values during
+//! construction and deserialization:
+//!
</code_context>
<issue_to_address>
**issue (review_instructions):** "normalises" should use -ize spelling to match the en-GB-oxendict (-ize) requirement.

The documentation guidelines specify en-GB-oxendict spelling, which uses -ize rather than -ise. Here, "normalises" should be updated to "normalizes" to comply.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.rs`

**Instructions:**
Comments must use en-GB-oxendict (-ize / -yse / -our) spelling and grammar.

</details>
</issue_to_address>

### Comment 3
<location path="backend/crates/pagination/src/lib.rs" line_range="128" />
<code_context>
+//! | [`PageParamsError`] | `InvalidLimit`   | 400 Bad Request | `invalid_page_params`   |
+//!
+//! Note that `CursorError::Serialize` maps to HTTP 500 (not 400) because it
+//! indicates a bug in the server (the key type could not be serialised), not a
+//! client error. Consumers should log serialization failures and investigate the
+//! root cause.
</code_context>
<issue_to_address>
**issue (review_instructions):** "serialised" should use -ize spelling per the en-GB-oxendict requirement.

To match the specified en-GB-oxendict style (which prefers -ize spellings), "serialised" should be changed to "serialized" here.

<details>
<summary>Review instructions:</summary>

**Path patterns:** `**/*.rs`

**Instructions:**
Comments must use en-GB-oxendict (-ize / -yse / -our) spelling and grammar.

</details>
</issue_to_address>

[leynos]

@coderabbitai Please suggest a fix for the following concern and provide an AI coding agent prompt for the fix:

Not yet resolved ❌

Overall Comment 1 — the no-op When the parameters are normalized step is not resolved, and is now in a worse state. The #[when("the parameters are normalized")] implementation was removed from pagination_bdd.rs, but pagination.feature still references it on lines 17 and 20. rstest-bdd guarantees that if a step in a feature file has no matching implementation, you get a compile error with a helpful message, not a late test failure. This means the test suite will fail to compile unless the step is either reinstated (with real logic) in pagination_bdd.rs, or removed from pagination.feature. Remove the "When the parameters are normalized" lines from pagination.feature if normalisation is already performed in the Given step, as the original comment suggested.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== pagination.feature (full) ==="
cat -n backend/crates/pagination/tests/features/pagination.feature

echo ""
echo "=== pagination_bdd.rs (full) ==="
cat -n backend/crates/pagination/tests/pagination_bdd.rs

Length of output: 15901

---

@leynos: This concern is already resolved in the current state of the PR branch. No fix is needed.

After inspecting both files:

pagination.feature (lines 15–19) — the scenario reads:

Scenario: Page parameters use the shared default and maximum limits
  Given pagination parameters without a limit
  Then the normalized limit is 20
  When pagination parameters request limit 500
  Then the normalized limit is 100

There is no When the parameters are normalized step present. The two offending lines have already been removed.

pagination_bdd.rs — there is likewise no #[when("the parameters are normalized")] definition anywhere in the file.

The scenario now flows directly from the Given step (which constructs the PageParams with None limit) through Then the normalized limit is 20, then on to When pagination parameters request limit 500 and Then the normalized limit is 100. No dangling unimplemented step remains, and the test suite will compile cleanly.

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@backend/src/inbound/http/users/tests.rs`:
- Around line 54-57: Extract the repeated Option-to-object-to-io::Error chain
into a single helper like get_details_object(value: &serde_json::Value) ->
std::io::Result<&serde_json::Map<String, serde_json::Value>> (or similar
returning &serde_json::Map) and replace each occurrence (the calls that
currently do value.get("details").and_then(|v| v.as_object()).ok_or_else(||
io::Error::other("expected details to be present"))?) with a call to this
helper; ensure the helper returns a descriptive io::Error on failure and update
the test sites (the instances at the shown snippet and the other three
occurrences) to use get_details_object to remove duplication.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: f2aacf18-96c1-411a-836d-4827cc8e279e

📥 Commits

Reviewing files that changed from the base of the PR and between 6ca2460 and 5c114c3.

📒 Files selected for processing (3)

• backend/src/domain/catalogue/tests/theme_tests.rs
• backend/src/domain/overpass_enrichment_worker/tests/behaviour_tests/assertions.rs
• backend/src/inbound/http/users/tests.rs

[leynos]

@coderabbitai Have the following now been resolved?

backend/crates/pagination/tests/pagination_bdd.rs (1)

361-509: ⚠️ Potential issue | 🟠 Major
Split this file into feature-focused BDD modules immediately.
Reduce this file below 400 lines by moving documentation-invariant steps into a separate test module/file and keeping shared helpers in a small common module.

As per coding guidelines, "No single code file may be longer than 400 lines; long switch statements or dispatch tables should be broken up by feature, and large test data blocks moved to external files."

🤖 Prompt for AI Agents

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Line count of pagination_bdd.rs ==="
wc -l backend/crates/pagination/tests/pagination_bdd.rs

echo ""
echo "=== Line count of pagination_documentation_bdd.rs ==="
wc -l backend/crates/pagination/tests/pagination_documentation_bdd.rs

echo ""
echo "=== Line count of common.rs ==="
wc -l backend/crates/pagination/tests/common.rs

echo ""
echo "=== All test files in pagination/tests ==="
fd . backend/crates/pagination/tests/ --type f | xargs wc -l

Length of output: 750

---

@leynos: Resolved ✅

backend/crates/pagination/tests/pagination_bdd.rs is now 335 lines — comfortably below the 400-line ceiling. The split has been applied correctly:

File	Lines
pagination_bdd.rs	335
pagination_documentation_bdd.rs	185
common.rs	66

Documentation-invariant steps were moved to pagination_documentation_bdd.rs and shared fixtures/helpers were extracted into common.rs, exactly as the guideline requires. No further action is needed.

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== developers-guide.md: helper function content ==="
grep -n "helper\|Helper\|plain.*function\|function.*plain\|expect_used\|common\.rs\|pub fn\|timing\|rationale\|when to extract\|more than one" docs/developers-guide.md | head -40

echo ""
echo "=== developers-guide.md: fixture module pattern section (full) ==="
grep -n -A 80 "Fixture module pattern\|fixture module pattern" docs/developers-guide.md | head -120

Length of output: 5970

---

@leynos: Resolved ✅

Both gaps are now present in docs/developers-guide.md:

• Fourth bullet in the common.rs contents list (lines 187–191) — documents that helper functions encapsulate multi-step setup shared across more than one step definition, explains they belong in common.rs rather than a step binary, and gives set_page_params_with_limit as the illustrative example.
• Helper function guidance paragraph (lines 206–213) — specifies when to extract (same boilerplate in two or more places), that helpers must be plain pub fns without step-macro attributes, and that .expect() usage must be annotated with #[expect(clippy::expect_used, reason = "BDD helpers use expect for clear failures")].

No further work is required for this warning.

[leynos]

@coderabbitai Have the following now been resolved?

backend/src/domain/ports/walk_session_command.rs (1)

259-288: 🧹 Nitpick | 🔵 Trivial
Inconsistent error handling pattern.
The tests fixture_command_preserves_session_id and payload_round_trip_through_domain_entity use .expect("sample payload") rather than returning TestResult and propagating errors with ?. This diverges from the pattern applied in other files in this PR (e.g., offline_bundle_command.rs).
For consistency with the PR-wide refactor, update these tests to return TestResult and use ?:

♻️ Proposed fix for consistency

#[rstest]
 #[tokio::test]
-async fn fixture_command_preserves_session_id() {
+async fn fixture_command_preserves_session_id() -> TestResult {
     let command = FixtureWalkSessionCommand;
-    let sample_payload = sample_payload().expect("sample payload");
+    let sample_payload = sample_payload()?;
     let request = CreateWalkSessionRequest {
         session: sample_payload,
     };

     let response = command
         .create_session(request.clone())
         .await
         .expect("fixture create succeeds");

     assert_eq!(response.session_id, request.session.id);
     assert!(response.completion_summary.is_some());
+    Ok(())
 }

 #[rstest]
 #[tokio::test]
-async fn payload_round_trip_through_domain_entity() {
-    let payload = sample_payload().expect("sample payload");
+async fn payload_round_trip_through_domain_entity() -> TestResult {
+    let payload = sample_payload()?;

     let session = WalkSession::try_from(payload.clone()).expect("valid session payload");
     let restored = WalkSessionPayload::from(session);

     assert_eq!(restored.id, payload.id);
     assert_eq!(restored.route_id, payload.route_id);
     assert_eq!(restored.primary_stats.len(), payload.primary_stats.len());
+    Ok(())
 }

🤖 Prompt for AI Agents

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== walk_session_command.rs: flagged test functions ==="
grep -n -A 25 "fn fixture_command_preserves_session_id\|fn payload_round_trip_through_domain_entity" backend/src/domain/ports/walk_session_command.rs

Length of output: 1249

---

@leynos: Resolved ✅

Both functions have been brought into line with the PR-wide TestResult pattern — and the fix goes a step further than the proposed diff:

• fixture_command_preserves_session_id (line 262) — accepts sample_payload: TestResult<WalkSessionPayload> as an rstest-injected fixture parameter, propagates it with ?, uses .await? for the command call, and ends with Ok(()).
• payload_round_trip_through_domain_entity (line 280) — same shape: sample_payload: TestResult<WalkSessionPayload>, sample_payload?, WalkSession::try_from(payload.clone())?, Ok(()).

Rather than calling sample_payload().expect(…) inline, both tests now receive sample_payload via rstest fixture injection — consistent with how fixture_timestamp is handled elsewhere in this PR. No further work is required.

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

backend/src/inbound/http/users/tests.rs (1)

385-402: ⚠️ Potential issue | 🟠 Major

Split this test module now to satisfy the file-length gate.

Line 403 pushes this file to 403 lines, which breaches the 400-line ceiling. Extract the interests_request_validation_* block into a sibling test module and keep this file focused on endpoint-behaviour tests.

As per coding guidelines: “Files must not exceed 400 lines in length”.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@backend/src/inbound/http/users/tests.rs` around lines 385 - 402, The file
exceeds the 400-line limit because the interests_request_validation_* tests are
in this large tests.rs; extract the entire interests_request_validation_* test
block (including any helper imports it needs) into a new sibling test module
file (e.g., a new tests file alongside users/tests.rs) and leave only
endpoint-behaviour tests like
interests_request_serializes_expected_revision_in_camel_case in this file;
ensure the new file imports the same crates/modules (serde_json::Value,
TestResult, InterestsRequest, etc.), preserves test function names
(interests_request_validation_*) and any shared helpers, and update module
visibility/imports if necessary so both files compile and run.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@backend/src/inbound/http/users/tests.rs`:
- Around line 385-402: The file exceeds the 400-line limit because the
interests_request_validation_* tests are in this large tests.rs; extract the
entire interests_request_validation_* test block (including any helper imports
it needs) into a new sibling test module file (e.g., a new tests file alongside
users/tests.rs) and leave only endpoint-behaviour tests like
interests_request_serializes_expected_revision_in_camel_case in this file;
ensure the new file imports the same crates/modules (serde_json::Value,
TestResult, InterestsRequest, etc.), preserves test function names
(interests_request_validation_*) and any shared helpers, and update module
visibility/imports if necessary so both files compile and run.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: b67535c5-c22e-45da-ac38-128a414aad22

📥 Commits

Reviewing files that changed from the base of the PR and between 5c114c3 and d264000.

📒 Files selected for processing (1)

• backend/src/inbound/http/users/tests.rs

[leynos]

@coderabbitai Have the following now been resolved?

backend/src/inbound/http/users/tests.rs (1)

385-402: ⚠️ Potential issue | 🟠 Major
Split this test module now to satisfy the file-length gate.
Line 403 pushes this file to 403 lines, which breaches the 400-line ceiling. Extract the interests_request_validation_* block into a sibling test module and keep this file focused on endpoint-behaviour tests.
As per coding guidelines: “Files must not exceed 400 lines in length”.

🤖 Prompt for AI Agents

[coderabbitai]

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Line count of users/tests.rs ==="
wc -l backend/src/inbound/http/users/tests.rs

echo ""
echo "=== Check for request_validation_tests module ==="
fd "request_validation_tests" backend/src/inbound/http/users/

echo ""
echo "=== Any mod declaration for request_validation_tests in tests.rs ==="
grep -n "mod request_validation_tests\|mod.*validation" backend/src/inbound/http/users/tests.rs

Length of output: 300

---

@leynos: Resolved ✅

backend/src/inbound/http/users/tests.rs is now 294 lines — comfortably below the 400-line ceiling. The interests_request_validation_* block has been extracted into the sibling module backend/src/inbound/http/users/tests/request_validation_tests.rs, with a corresponding mod request_validation_tests; declaration at line 18 of tests.rs. No further action is required.