Skip to content

Revision-safe interests; CI wiring; remove pg_worker; create dest dir#323

Merged
leynos merged 15 commits intomainfrom
implement-revision-safe-update-strategy-i43x3p
Mar 18, 2026
Merged

Revision-safe interests; CI wiring; remove pg_worker; create dest dir#323
leynos merged 15 commits intomainfrom
implement-revision-safe-update-strategy-i43x3p

Conversation

@leynos
Copy link
Copy Markdown
Owner

@leynos leynos commented Mar 13, 2026

Summary

  • Implement revision-safe interests update strategy (3.5.4) and related backend wiring, aligning with existing revision-concurrency patterns across the API. This update also includes CI/test infra tweaks to reliably provision and copy the pg_worker binary during tests.

  • Create destination directory before copying pg_worker to ensure reliable placement of the binary in test/CI environments.

  • Remove legacy in-tree pg_worker binary (backend/src/bin/pg_worker.rs) as tests now rely on the embedded/installed worker provided by pg-embed-setup-unpriv.

  • Update CI workflow (.github/workflows/ci.yml) to invoke make prepare-pg-worker in place of direct cargo build for the worker, ensuring the new directory-prep step runs.

  • Updated: The Makefile to support a robust pg_worker setup:

    • PG_WORKER_PATH remains the destination for the worker binary.
    • Create the destination directory prior to copying the binary.
    • If a pg_worker exists on PATH, copy it; otherwise install pg-embed-setup-unpriv's pg_worker into a build-root and copy it to the destination.
    • Introduced PG_WORKER_INSTALL_ROOT and related logic to source the embedded binary when PATH is not available.
  • Changes to tests and adapters to reflect the typed UpdateUserInterestsRequest and revision-based flow.

  • Documentation and execplan artifacts for 3.5.4 revision-safe interests, including updated architecture notes and a new execplan doc.

  • Updated: Several test doubles and harnesses to support the new typed request and revision-encoded flow (UpdateUserInterestsRequest in tests and mocks).

  • OpenAPI/Docs & Other Artifacts

    • Added docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md detailing the revision-safe semantics and rationale.
    • Updated docs to reflect that interests writes share the same revision as full preferences and must preserve non-interest fields.
    • Adjusted wildside-backend-architecture.md and wildside-pwa-data-model.md to reflect the revised contract.

Rationale

  • The new flow aligns interests with the existing revision-based concurrency model already used for preferences, routes notes, and progress. Clients can supply an optional expectedRevision, and conflicts are surfaced as HTTP 409 with structured details (revision_mismatch) to enable safe retries.
  • The tests and DB harness leverage embedded PostgreSQL via pg-embed-setup-unpriv, so ensuring the destination directory exists before copying the binary eliminates a class of CI/test-time failures. Falling back to PATH-available binaries when present keeps CI and local dev consistent.
  • Removing the legacy in-tree pg_worker binary ensures tests rely on the embedded/install mechanism, removing maintenance debt and potential drift.

Validation & Acceptance Criteria

  • The destination directory for pg_worker is created prior to copy in all relevant scripts.
  • pg_worker is sourced from PATH when available or installed into a known root and copied to the desired path, with the destination directory guaranteed to exist.
  • The existing revision-safe interests update path remains behaviorally unchanged for domain, persistence, and HTTP layers, with added robustness in pg_worker setup.
  • OpenAPI specs, tests, and docs reflect the revision-safe contract and updated payloads.

OpenAPI/Docs & Other Artifacts

  • The OpenAPI payloads and schemas were updated to include the revision token and to expose the updated response payload with revision.
  • New 3.5.4 execplan docs describe the revision-safe updates and the test harness changes.

Prior Art / References

Next Steps

  • Run full test suite locally and in CI to confirm stable behavior for revision-safe flows and to validate that the destination directory issue is resolved.
  • Verify that all affected tests (unit, adapter guardrails, and DB-backed BDD flows) pass with embedded PostgreSQL.

📎 Task: https://www.devboxer.com/task/d81ac072-93a0-4fdd-ad2a-7a435c5516a7
📎 Task: https://www.devboxer.com/task/92fae8db-ed86-44a8-b7ea-f51aa3be4824

…tegy

Add a detailed 616-line execution plan for roadmap item 3.5.4 defining and implementing a revision-safe update strategy for user interests.

This new document outlines the purpose, constraints, tolerances, risks, progress stages, surprises, decision log, outcomes, plan of work, interfaces, error mapping, and team ownership related to replacing the legacy last-write-wins interests update with an optimistic concurrency model aligned with the existing user preferences aggregate.

No implementation is included yet; this draft locks the architectural and concurrency decisions to guide subsequent development and testing.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
@sourcery-ai
Copy link
Copy Markdown

sourcery-ai Bot commented Mar 13, 2026

Reviewer's Guide

Adds an ExecPlan markdown document defining the revision-safe update strategy for PUT /api/v1/users/me/interests, including goals, constraints, risks, progress tracking, and a staged implementation plan, without changing any production code.

Sequence diagram for revision-safe PUT /api/v1/users/me/interests

sequenceDiagram
    actor Client
    participant HttpUsersHandler
    participant UserInterestsCommand
    participant DieselUserInterestsCommand
    participant UserPreferencesRepository
    participant Postgres

    Client->>HttpUsersHandler: PUT /api/v1/users/me/interests
    activate HttpUsersHandler
    HttpUsersHandler->>HttpUsersHandler: parse_InterestsRequest_with_expectedRevision
    HttpUsersHandler->>UserInterestsCommand: update(UpdateUserInterestsRequest)
    activate UserInterestsCommand

    UserInterestsCommand->>DieselUserInterestsCommand: delegate_update(UpdateUserInterestsRequest)
    activate DieselUserInterestsCommand

    DieselUserInterestsCommand->>UserPreferencesRepository: find_by_user_id(UserId)
    activate UserPreferencesRepository
    UserPreferencesRepository->>Postgres: SELECT_from_user_preferences
    Postgres-->>UserPreferencesRepository: row_or_none_with_revision
    UserPreferencesRepository-->>DieselUserInterestsCommand: UserPreferencesOption
    deactivate UserPreferencesRepository

    alt no_existing_row_and_expected_revision_none
        DieselUserInterestsCommand->>UserPreferencesRepository: save(new_row_revision_1)
        UserPreferencesRepository->>Postgres: INSERT_user_preferences_rev_1
        Postgres-->>UserPreferencesRepository: insert_ok
        UserPreferencesRepository-->>DieselUserInterestsCommand: saved_preferences_rev_1
        DieselUserInterestsCommand-->>UserInterestsCommand: Ok(UserInterests_rev_1)
    else no_existing_row_and_expected_revision_some
        DieselUserInterestsCommand-->>UserInterestsCommand: Err(conflict_revision_mismatch_actual_0)
    else existing_row_and_expected_revision_none
        DieselUserInterestsCommand-->>UserInterestsCommand: Err(conflict_missing_expected_revision)
    else existing_row_and_revision_mismatch
        DieselUserInterestsCommand-->>UserInterestsCommand: Err(conflict_revision_mismatch_actual_current)
    else existing_row_and_revision_matches
        DieselUserInterestsCommand->>UserPreferencesRepository: save(updated_row_revision_plus_1)
        UserPreferencesRepository->>Postgres: UPDATE_user_preferences_set_interests_increment_revision
        Postgres-->>UserPreferencesRepository: update_ok
        UserPreferencesRepository-->>DieselUserInterestsCommand: saved_preferences_new_revision
        DieselUserInterestsCommand-->>UserInterestsCommand: Ok(UserInterests_new_revision)
    end

    deactivate DieselUserInterestsCommand
    UserInterestsCommand-->>HttpUsersHandler: UserInterests_or_conflict_error
    deactivate UserInterestsCommand

    alt success
        HttpUsersHandler-->>Client: 200_OK_JSON_with_interestThemeIds_and_revision
    else stale_write_conflict
        HttpUsersHandler-->>Client: 409_Conflict_JSON_code_conflict_details_revision_mismatch
    end
    deactivate HttpUsersHandler
Loading

Class diagram for revision-safe interests domain and adapter contracts

classDiagram
    class UpdateUserInterestsRequest {
        +UserId user_id
        +Vec~InterestThemeId~ interest_theme_ids
        +Option~u32~ expected_revision
    }

    class UserInterests {
        +UserId user_id
        +Vec~InterestThemeId~ interest_theme_ids
        +u32 revision
    }

    class Error {
        +code: String
        +details: String
    }

    class UserInterestsCommand {
        <<interface>>
        +update(request UpdateUserInterestsRequest) Result~UserInterests, Error~
    }

    class UserPreferencesRepository {
        <<interface>>
        +find_by_user_id(user_id UserId) Result~Option~UserPreferences~, UserPreferencesRepositoryError~
        +save(preferences UserPreferences, expected_revision Option~u32~) Result~UserPreferences, UserPreferencesRepositoryError~
    }

    class UserPreferencesRepositoryError {
        +kind: String
        +message: String
    }

    class UserPreferences {
        +UserId user_id
        +Vec~InterestThemeId~ interest_theme_ids
        +Vec~SafetyToggleId~ safety_toggle_ids
        +UnitSystem unit_system
        +u32 revision
    }

    class DieselUserInterestsCommand {
        +repository: UserPreferencesRepository
        +update(request UpdateUserInterestsRequest) Result~UserInterests, Error~
        -handle_insert_case(request UpdateUserInterestsRequest) Result~UserInterests, Error~
        -handle_update_case(request UpdateUserInterestsRequest, preferences UserPreferences) Result~UserInterests, Error~
        -map_repository_error(error UserPreferencesRepositoryError) Error
    }

    class HttpUsersHandler {
        +update_interests(request_body InterestsRequest, user_id UserId) HttpResponse
    }

    class InterestsRequest {
        +Vec~InterestThemeId~ interest_theme_ids
        +Option~u32~ expected_revision
    }

    class HttpResponse {
        +status: u16
        +body: String
    }

    UserInterestsCommand <|.. DieselUserInterestsCommand
    DieselUserInterestsCommand --> UserPreferencesRepository
    UserPreferencesRepository --> UserPreferences
    HttpUsersHandler --> UserInterestsCommand
    HttpUsersHandler --> InterestsRequest
    HttpUsersHandler --> HttpResponse
    UpdateUserInterestsRequest --> UserInterests
    UserPreferences --> UserInterests
    UserPreferencesRepositoryError --> Error
Loading

Flow diagram for revision-safe interests persistence decision matrix

flowchart TD
    A[Start_update_interests] --> B[Load_user_preferences_by_user_id]
    B --> C{Preferences_row_exists}

    C -->|no| D{expected_revision_is_some}
    D -->|no| E[Insert_new_preferences_row_with_interests_revision_1]
    E --> F[Return_UserInterests_revision_1]

    D -->|yes| G[Return_conflict_error_actualRevision_0_expectedRevision_supplied]

    C -->|yes| H{expected_revision_is_some}

    H -->|no| I[Return_conflict_error_missing_expectedRevision_for_existing_row]

    H -->|yes| J{expected_revision_equals_current_revision}

    J -->|no| K[Return_conflict_error_revision_mismatch]

    J -->|yes| L[Update_existing_row_set_interests_preserve_safety_toggle_ids_and_unit_system_increment_revision]
    L --> M[Return_UserInterests_with_new_revision]

    F --> N[End]
    G --> N
    I --> N
    K --> N
    M --> N
Loading

File-Level Changes

Change Details Files
Add a comprehensive ExecPlan document describing how to implement revision-safe, optimistic-concurrency updates for the interests endpoint.
  • Introduce a living execution plan with sections for constraints, tolerances, risks, progress, surprises, decision log, and outcomes.
  • Specify the target behaviour for PUT /api/v1/users/me/interests using an expectedRevision-based optimistic concurrency model aligned with existing preferences and notes endpoints.
  • Detail a staged implementation plan across domain ports, persistence adapters, HTTP/OpenAPI contracts, tests (rstest and rstest-bdd), and documentation updates.
  • Define concrete validation and acceptance criteria, including HTTP semantics, persistence rules with user_preferences.revision, and required test/lint/doc gates.
  • Document ownership via an agent-team model, handoff order, and concrete shell commands/logging practices for executing and validating the work.
docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.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

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 13, 2026

Summary by CodeRabbit

Release Notes

  • New Features

    • Added revision-safe updates for user interests with optimistic concurrency control to prevent lost updates during concurrent operations.
    • Response payloads now include revision numbers for tracking.
    • Enhanced conflict detection with detailed revision information in HTTP 409 responses.
  • Documentation

    • Updated architecture documentation to clarify revision-aware preference update semantics and conflict handling.
  • Chores

    • Improved build system for pg_worker binary preparation.

Walkthrough

This PR introduces revision-safe user interests updates with optimistic concurrency control throughout the backend stack. The pg_worker binary is removed from Cargo and refactored to a Makefile-based build approach. Comprehensive BDD test coverage validates revision conflict scenarios against a real database.

Changes

Cohort / File(s) Summary
Build Infrastructure
.github/workflows/ci.yml, Makefile
Removed direct Cargo build commands for pg_worker; introduced PG_WORKER_INSTALL_ROOT and PG_EMBED_SETUP_UNPRIV_VERSION variables. The prepare-pg-worker target now checks PATH for pg_worker availability and falls back to cargo installation into a cache directory.
pg_worker Binary Removal
backend/Cargo.toml, backend/src/bin/pg_worker.rs
Deleted the [[bin]] target and entire 152-line pg_worker.rs implementation, including operation enum, payload loading, and argument parsing logic.
Domain Layer & DTOs
backend/src/domain/user_interests.rs, backend/src/domain/ports/user_interests_command.rs, backend/src/domain/ports/mod.rs
Added revision: u32 field to UserInterests. Introduced UpdateUserInterestsRequest DTO with user_id, interest_theme_ids, and optional expected_revision. Updated UserInterestsCommand::set_interests signature to accept UpdateUserInterestsRequest.
HTTP Layer
backend/src/inbound/http/users.rs, backend/src/inbound/http/users/tests.rs, backend/src/inbound/http/schemas.rs
Added expected_revision field to InterestsRequest. Introduced ParsedInterests internal type. Added revision field to UserInterestsSchema. Updated endpoint to document 409 Conflict response and construct UpdateUserInterestsRequest with optional expected_revision.
Persistence Refactoring
backend/src/outbound/persistence/diesel_user_interests_command.rs, backend/src/outbound/persistence/diesel_user_interests_command/tests/...
Replaced retry-loop logic with revision-aware single-path handling. Introduced revision_conflict helper. Updated set_interests to accept UpdateUserInterestsRequest. Removed MAX_CONCURRENT_WRITE_ATTEMPTS, PreferencesUpdate, and retry infrastructure. Consolidated error mapping for RevisionMismatch and MissingForUpdate to return HTTP 409.
Test Fixture & Support
backend/tests/adapter_guardrails/doubles_users.rs, backend/tests/adapter_guardrails/harness_defaults.rs, backend/tests/user_session_bdd.rs, backend/tests/user_state_profile_interests_startup_modes_bdd.rs
Updated test doubles and assertions to use UpdateUserInterestsRequest and validate revision. Added revision parameter to UserInterests construction. Updated assertion helpers to check response revision field.
BDD Test Suite
backend/tests/features/user_interests_revision_conflicts.feature, backend/tests/user_interests_revision_conflicts_bdd.rs, backend/tests/user_interests_revision_conflicts_bdd/db_support.rs, backend/tests/user_interests_revision_conflicts_bdd/flow_support.rs
New 240+ line BDD test module with feature scenarios validating revision creation, increment, staleness detection, and missing revision conflicts. Introduced DbContext, SeedPreferences, World, and comprehensive flow and assertion helpers for end-to-end testing against embedded PostgreSQL.
Lint Configuration
backend/tests/{admin_enrichment_reporting_bdd,catalogue_endpoints_bdd,offline_walk_endpoints_bdd,pwa_annotations_bdd,pwa_preferences_bdd}.rs
Removed clippy::type_complexity #[expect(...)] attributes from five test modules; dead_code #[expect(...)] remains in place.
Configuration & Dependencies
package.json, biome.jsonc
Added pnpm override for yauzl 3.2.1. Removed noImportCycles error rule from biome nursery linter.
Architecture & Testing Documentation
docs/wildside-backend-architecture.md, docs/wildside-pwa-data-model.md, docs/wildside-testing-guide.md, docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md, docs/investigations/2026-03-13-toolchain-errors.md
Updated architecture documentation to clarify interests writes update user_preferences in place with shared revision semantics. Added UserInterests type definition to data model. Documented pg_worker build behaviour. Added comprehensive 632-line ExecPlan detailing revision-safe strategy, stages, and acceptance criteria. Included 505-line investigation report on three transient toolchain failures encountered during development.
sequenceDiagram
    actor Client
    participant HTTP as HTTP Handler
    participant Domain as Domain<br/>(UpdateUserInterestsRequest)
    participant Persist as Persistence<br/>(DieselUserInterestsCommand)
    participant DB as Database<br/>(user_preferences)

    Client->>HTTP: PUT /interests<br/>(expected_revision: 2)
    HTTP->>HTTP: Parse expected_revision<br/>into UpdateUserInterestsRequest
    HTTP->>Persist: set_interests(request)
    Persist->>DB: SELECT user_preferences<br/>WHERE user_id = ?
    DB-->>Persist: existing revision: 1
    alt Revision Mismatch
        Persist->>Persist: Build conflict error<br/>(expected: 2, actual: 1)
        Persist-->>HTTP: Err(Conflict {<br/>expectedRevision: 2,<br/>actualRevision: 1 })
        HTTP-->>Client: 409 Conflict
    else Revision Match
        Persist->>Persist: Calculate next<br/>revision: 2 + 1 = 3
        Persist->>DB: UPDATE user_preferences<br/>SET interest_theme_ids = ?,<br/>revision = 3<br/>WHERE user_id = ? AND revision = 2
        DB-->>Persist: OK
        Persist-->>HTTP: UserInterests<br/>{revision: 3}
        HTTP-->>Client: 200 OK<br/>{revision: 3}
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~55 minutes

Possibly related PRs

Poem

🔄 Revisions rise in careful steps,
Concurrency no longer stumbles;
Optimistic locks and conflict checks,
As the database faithfully tumbles.
pg_worker bids farewell to Cargo's shore,
Make prepares the way once more. 🏗️

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 70.09% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately summarises the primary changes: implementing revision-safe interests updates, updating CI workflow, removing the legacy pg_worker binary, and ensuring the destination directory is created.
Description check ✅ Passed The description comprehensively covers the changeset, detailing revision-safe interests implementation, CI/test infrastructure updates, pg_worker removal, destination directory creation, and documentation changes.

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

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch implement-revision-safe-update-strategy-i43x3p
📝 Coding Plan
  • Generate coding plan for human review comments

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

codescene-delta-analysis[bot]

This comment was marked as outdated.

…s update

- Introduce UpdateUserInterestsRequest to carry expectedRevision for concurrency control.
- UserInterests now includes a revision field to track aggregate version.
- Convert DieselUserInterestsCommand to enforce revision checks and reject stale writes.
- Expose revision-safe update and conflict semantics in HTTP API for PUT /users/me/interests.
- Remove implicit retries on concurrency conflicts; surface 409 Conflict with revision details.
- Add extensive unit and integration tests covering revision conflicts and concurrency handling.
- Update documentation to reflect that interests share aggregate and revision with user preferences, with optimistic locking.

This change ensures safe concurrent updates on user interests by leveraging the aggregate revision from user preferences, providing explicit conflict feedback to clients when revisions are stale or missing after the first write.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
@leynos leynos changed the title Define ExecPlan: revision-safe interests update (3.5.4) Implement backend revision-safe interests update (3.5.4) Mar 13, 2026
codescene-delta-analysis[bot]

This comment was marked as outdated.

…errors and failures

Add a comprehensive 500-line investigation document describing three transient
failures observed in the backend validation process. This report includes
failure reproductions, environment snapshots, failure diagnoses, relevant
code paths, and recommended next steps. The failures are analyzed in detail,
identifying causes such as a broken /dev/null device and transient toolchain
instabilities, with evidence ruling out persistent code defects.

This addition aids in debugging and future failure mitigation related to the
Rust toolchain and container environment.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 13, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

Excess Number of Function Arguments

backend/tests/user_interests_revision_conflicts_bdd/db_support.rs: seed_preferences

What lead to degradation?

seed_preferences has 6 arguments, max arguments = 4

Why does this problem occur?

Functions with many arguments indicate either a) low cohesion where the function has too many responsibilities, or b) a missing abstraction that encapsulates those arguments.
The threshold for the Rust language is 4 function arguments.

How to fix it?

Start by investigating the responsibilities of the function. Make sure it doesn't do too many things, in which case it should be split into smaller and more cohesive functions. Consider the refactoring INTRODUCE PARAMETER OBJECT to encapsulate arguments that refer to the same logical concept.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 13, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

Code Duplication

backend/tests/user_interests_revision_conflicts_bdd/flow_support.rs:

What lead to degradation?

The module contains 5 functions with similar structure: preferences_snapshot,run_first_write,run_missing_revision_conflict,run_stale_revision_conflict and 1 more functions

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 13, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

Code Duplication

backend/tests/user_interests_revision_conflicts_bdd.rs:

What lead to degradation?

The module contains 6 functions with similar structure: existing_preferences_revision_1_with_preserved_safety_and_unit_settings,existing_preferences_revision_2,the_first_interests_response_includes_revision_1,the_response_is_a_conflict_with_expected_revision_1_and_actual_revision_2 and 2 more functions

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 13, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

Code Duplication

backend/tests/user_interests_revision_conflicts_bdd/flow_support.rs:

What lead to degradation?

The module contains 5 functions with similar structure: preferences_snapshot,run_first_write,run_missing_revision_conflict,run_stale_revision_conflict and 1 more functions

Why does this problem occur?

Duplicated code often leads to code that's harder to change since the same logical change has to be done in multiple functions. More duplication gives lower code health.

How to fix it?

A certain degree of duplicated code might be acceptable. The problems start when it is the same behavior that is duplicated across the functions in the module, ie. a violation of the Don't Repeat Yourself (DRY) principle. DRY violations lead to code that is changed together in predictable patterns, which is both expensive and risky. DRY violations can be identified using CodeScene's X-Ray analysis to detect clusters of change coupled functions with high code similarity. Read More
Once you have identified the similarities across functions, look to extract and encapsulate the concept that varies into its own function(s). These shared abstractions can then be re-used, which minimizes the amount of duplication and simplifies change.

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

This change removes the local `backend/src/bin/pg_worker.rs` binary and the corresponding entry in `Cargo.toml`. Instead, the Makefile is updated to use the `pg_worker` binary provided by the `pg-embed-setup-unpriv` crate.

- Makefile updated to check for an existing `pg_worker` on PATH and install it if missing.
- Introduced `PG_WORKER_INSTALL_ROOT` and `PG_EMBED_SETUP_UNPRIV_VERSION` variables in Makefile.
- Documentation updated to describe the new usage pattern for the embedded Postgres worker helper binary.

This simplifies maintenance by relying on an external, versioned binary rather than one built and maintained in this repository.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
@leynos leynos changed the title Implement backend revision-safe interests update (3.5.4) Implement backend revision-safe interests update strategy (3.5.4) Mar 13, 2026
codescene-delta-analysis[bot]

This comment was marked as outdated.

…flow and DB support for user interests revision

- Extracted common test logic into helper functions (e.g., with_world, seed_prefs) to reduce code duplication and improve readability.
- Modified seed_preferences DB support function to accept user_id as explicit parameter enhancing clarity.
- Simplified test step implementations by wrapping them with helper functions managing skip checks.
- Refined flow_support functions to unify update operations and introduce reusable session handling (with_app_session), improving async control and reducing boilerplate.
- Cleaned up conditional checks scattered in tests and standardized update call patterns.

These changes improve maintainability and clarity of BDD test code for user preferences revision conflict scenarios.

Co-authored-by: devboxerhub[bot] <devboxerhub[bot]@users.noreply.github.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 13, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

backend/tests/user_interests_revision_conflicts_bdd.rs

Comment on file

fn the_first_interests_response_includes_revision_1(world: &mut World) {
    with_world(world, |world| {
        assert_interests_snapshot(
            world.first_update.as_ref().expect("first update response"),
            &[FIRST_THEME_ID],
            1,
        );
    });
}

❌ New issue: Code Duplication
The module contains 4 functions with similar structure: the_first_interests_response_includes_revision_1,the_response_is_a_conflict_with_expected_revision_1_and_actual_revision_2,the_response_is_a_conflict_with_missing_expected_revision_and_actual_revision_1,the_second_interests_response_includes_revision_2

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 13, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

backend/tests/user_interests_revision_conflicts_bdd.rs

Comment on file

fn seed_prefs(
    world: &mut World,
    interests: &[&str],
    safety: &[&str],
    unit_system: &str,
    revision: i32,
) {
    let db = world.db.as_ref().expect("db context");
    let user_id = Uuid::parse_str(FIXTURE_AUTH_ID).expect("valid fixture UUID");
    seed_preferences(
        db.database_url.as_str(),
        user_id,
        SeedPreferences {
            interest_ids: interests,
            safety_ids: safety,
            unit_system,
            revision,
        },
    )
    .expect("seed user preferences");
}

❌ New issue: Excess Number of Function Arguments
seed_prefs has 5 arguments, max arguments = 4

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 13, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

backend/tests/user_interests_revision_conflicts_bdd/flow_support.rs

Comment on file

async fn update_interests_snapshot<S>(
    app: &S,
    cookie: Cookie<'static>,
    payload: InterestsRequest,
) -> Snapshot
where
    S: actix_web::dev::Service<
            actix_http::Request,
            Response = actix_web::dev::ServiceResponse<actix_web::body::BoxBody>,
            Error = actix_web::Error,
        >,
{
    let req = actix_test::TestRequest::put()
        .uri("/api/v1/users/me/interests")
        .cookie(cookie)
        .set_json(payload)
        .to_request();
    capture_snapshot(actix_test::call_service(app, req).await, false).await
}

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: preferences_snapshot,update_interests_snapshot

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

@coderabbitai

This comment was marked as resolved.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos leynos marked this pull request as ready for review March 14, 2026 15:31
Copy link
Copy Markdown

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @leynos, you have reached your weekly rate limit of 2500000 diff characters.

Please try again later or upgrade to continue using Sourcery

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7d9d7706b4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread Makefile
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 7

Caution

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

⚠️ Outside diff range comments (2)
backend/src/inbound/http/schemas.rs (1)

200-210: 🧹 Nitpick | 🔵 Trivial

Extend schema test to verify the new revision field.

The test asserts interestThemeIds is present but does not check for the newly added revision field.

♻️ Proposed fix
     fn user_interests_schema_has_expected_name() {
         let schema_json = schema_to_json::<UserInterestsSchema>();
         let name = UserInterestsSchema::name();
         // utoipa replaces :: with . in schema names
         assert_eq!(name, "crate.domain.UserInterests");
         assert!(
             schema_json.contains("interestThemeIds"),
             "schema should contain interestThemeIds field"
         );
+        assert!(
+            schema_json.contains("revision"),
+            "schema should contain revision field"
+        );
     }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/src/inbound/http/schemas.rs` around lines 200 - 210, The test
user_interests_schema_has_expected_name currently checks schema_json for
"interestThemeIds" but misses the newly added "revision" field; update this test
(in the function user_interests_schema_has_expected_name) to also assert that
schema_to_json::<UserInterestsSchema>() contains "revision" (or the exact JSON
key used for the revision field) so the test verifies both interestThemeIds and
revision are present in UserInterestsSchema's JSON schema.
backend/tests/diesel_profile_interests_adapters.rs (1)

167-184: ⚠️ Potential issue | 🟡 Minor

Add skip_serializing_if to properly test field omission.

The expected_revision field must be marked with #[serde(skip_serializing_if = "Option::is_none")] in the InterestsRequest struct. Without this, tests serialise None as "expectedRevision": null instead of omitting the field entirely, so the omission path remains unexercised.

Alternatively, build test payloads as raw JSON strings when asserting the omission behaviour.

Also applies to test cases at lines 302-305, 349-352.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@backend/tests/diesel_profile_interests_adapters.rs` around lines 167 - 184,
The test serialises InterestsRequest with expected_revision = None but currently
serialises as "expectedRevision": null; update the InterestsRequest definition
to annotate the expected_revision field with #[serde(skip_serializing_if =
"Option::is_none")] so that None values are omitted (or, if you prefer not to
change the struct, adjust the tests that call do_update_interests to send raw
JSON payloads that omit expectedRevision for the omission path); target the
InterestsRequest struct and its expected_revision field referenced by the tests
(also update the related test cases exercising omission).
🤖 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/domain/user_interests.rs`:
- Around line 20-21: Add short Rustdoc examples for the public API: include
example code for UserInterests::new that constructs a UserInterests (provide
sample UserId and InterestThemeId values) and asserts the revision returned by
UserInterests::revision(); also add a separate doc example on the revision()
method showing it returns the expected u32. Place these examples in the doc
comments for the new function and the revision() method so they demonstrate
construction and assert the revision value.

In `@backend/src/inbound/http/users/tests.rs`:
- Around line 358-370: The test is asserting HTTP camelCase on the domain DTO
(UpdateUserInterestsRequest) instead of the HTTP DTO; change the test to
serialize the inbound HTTP type InterestsRequest (the DTO consumed by
update_interests) and assert that its JSON uses "expectedRevision" with value 3,
removing the camelCase assertion from any domain types (e.g.,
UpdateUserInterestsRequest / crate::domain::ports) so the wire contract is
tested at the HTTP boundary handled by update_interests.

In `@docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md`:
- Around line 223-230: The documentation bullet describing revision-mismatch
responses is inconsistent with the implemented contract; update the sentence
under the PUT /api/v1/users/me/interests / DieselUserInterestsCommand notes to
state the conflict envelope shape used by the implementation: top-level code:
"conflict" with nested revision details (e.g. expectedRevision and
actualRevision) rather than top-level code: "revision_mismatch"; make this
wording match the validation section already in the doc so the ExecPlan and the
implementation are consistent.

In `@docs/investigations/2026-03-13-toolchain-errors.md`:
- Around line 286-287: Replace first-person narration in the specified prose
(e.g., phrases like "I re-ran…", "I also re-ran…", "I therefore assign…") with
neutral technical voice; update the sentences in the ranges referenced (lines
around 286-287, 311-313, 408-409, 466-467) to passive or third‑person
formulations such as "The probe was re-run…" or "The result is therefore
assigned…" so the document follows the guideline avoiding 1st/2nd person
pronouns.
- Line 1: The document uses title case for multiple headings (e.g., "# Toolchain
Error Investigation: 2026-03-13", "## Environment Snapshot", "## Relevant Code
Paths"); update each heading to sentence case across the file (including the
listed occurrences at lines referenced in the review) so they read like normal
sentences (for example, change "Toolchain Error Investigation: 2026-03-13" to
"Toolchain error investigation: 2026-03-13" and similarly convert "Environment
Snapshot" and "Relevant Code Paths" to "Environment snapshot" and "Relevant code
paths"); scan the whole file and replace all other headings mentioned in the
comment with their sentence-case equivalents to comply with the guideline.

In `@docs/wildside-backend-architecture.md`:
- Around line 475-482: Update the paragraph describing the 409 response for PUT
/api/v1/users/me/interests to reflect the actual envelope shape used by the
code/tests: top-level code must be "conflict" and the nested details must
include code: "revision_mismatch" plus the expectedRevision and actualRevision
fields; adjust the sentence that currently says code: "revision_mismatch" at top
level to instead describe details.code and include that the response contains
those revision fields to match the persistence regression tests and client
contract.

In `@docs/wildside-pwa-data-model.md`:
- Around line 384-401: Update the document metadata header in
docs/wildside-pwa-data-model.md by changing the "Last updated: 14 December 2025"
timestamp to the current date for this change; locate the top-of-file header
line that starts with "Last updated:" and replace the old date so the document
update that modifies the PUT /api/v1/users/me/interests contract is reflected in
the file metadata.

---

Outside diff comments:
In `@backend/src/inbound/http/schemas.rs`:
- Around line 200-210: The test user_interests_schema_has_expected_name
currently checks schema_json for "interestThemeIds" but misses the newly added
"revision" field; update this test (in the function
user_interests_schema_has_expected_name) to also assert that
schema_to_json::<UserInterestsSchema>() contains "revision" (or the exact JSON
key used for the revision field) so the test verifies both interestThemeIds and
revision are present in UserInterestsSchema's JSON schema.

In `@backend/tests/diesel_profile_interests_adapters.rs`:
- Around line 167-184: The test serialises InterestsRequest with
expected_revision = None but currently serialises as "expectedRevision": null;
update the InterestsRequest definition to annotate the expected_revision field
with #[serde(skip_serializing_if = "Option::is_none")] so that None values are
omitted (or, if you prefer not to change the struct, adjust the tests that call
do_update_interests to send raw JSON payloads that omit expectedRevision for the
omission path); target the InterestsRequest struct and its expected_revision
field referenced by the tests (also update the related test cases exercising
omission).
🪄 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: a464fa6b-1587-418b-9ad1-8fe5a9376f3e

📥 Commits

Reviewing files that changed from the base of the PR and between 1b3322e and 7d9d770.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (38)
  • .github/workflows/ci.yml
  • Makefile
  • backend/Cargo.toml
  • backend/src/bin/pg_worker.rs
  • backend/src/domain/ports/mod.rs
  • backend/src/domain/ports/user_interests_command.rs
  • backend/src/domain/user_interests.rs
  • backend/src/inbound/http/schemas.rs
  • backend/src/inbound/http/users.rs
  • backend/src/inbound/http/users/tests.rs
  • backend/src/outbound/persistence/diesel_user_interests_command.rs
  • backend/src/outbound/persistence/diesel_user_interests_command/tests.rs
  • backend/src/outbound/persistence/diesel_user_interests_command/tests/fixture_guards.rs
  • backend/src/outbound/persistence/diesel_user_interests_command/tests/mapping.rs
  • backend/src/outbound/persistence/diesel_user_interests_command/tests/retry.rs
  • backend/src/outbound/persistence/diesel_user_interests_command/tests/support.rs
  • backend/tests/adapter_guardrails/doubles_users.rs
  • backend/tests/adapter_guardrails/harness_defaults.rs
  • backend/tests/admin_enrichment_reporting_bdd.rs
  • backend/tests/catalogue_endpoints_bdd.rs
  • backend/tests/diesel_profile_interests_adapters.rs
  • backend/tests/features/user_interests_revision_conflicts.feature
  • backend/tests/offline_walk_endpoints_bdd.rs
  • backend/tests/pwa_annotations_bdd.rs
  • backend/tests/pwa_preferences_bdd.rs
  • backend/tests/user_interests_revision_conflicts_bdd.rs
  • backend/tests/user_interests_revision_conflicts_bdd/db_support.rs
  • backend/tests/user_interests_revision_conflicts_bdd/flow_support.rs
  • backend/tests/user_session_bdd.rs
  • backend/tests/user_state_profile_interests_startup_modes_bdd.rs
  • backend/tests/user_state_profile_interests_startup_modes_bdd/flow_support.rs
  • docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md
  • docs/investigations/2026-03-13-toolchain-errors.md
  • docs/wildside-backend-architecture.md
  • docs/wildside-pwa-data-model.md
  • docs/wildside-testing-guide.md
  • frontend-pwa/scripts/run-audit.test.mjs
  • package.json
💤 Files with no reviewable changes (9)
  • backend/tests/admin_enrichment_reporting_bdd.rs
  • backend/src/outbound/persistence/diesel_user_interests_command/tests/fixture_guards.rs
  • backend/tests/pwa_annotations_bdd.rs
  • backend/tests/catalogue_endpoints_bdd.rs
  • backend/tests/offline_walk_endpoints_bdd.rs
  • backend/tests/pwa_preferences_bdd.rs
  • backend/Cargo.toml
  • backend/src/outbound/persistence/diesel_user_interests_command/tests.rs
  • backend/src/bin/pg_worker.rs

Comment thread backend/src/domain/user_interests.rs
Comment thread docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md Outdated
Comment thread docs/investigations/2026-03-13-toolchain-errors.md Outdated
Comment thread docs/investigations/2026-03-13-toolchain-errors.md Outdated
Comment thread docs/wildside-backend-architecture.md Outdated
Comment thread docs/wildside-pwa-data-model.md
When pg_worker is found on PATH, the prepare-pg-worker target copies it
to target/pg_worker without first creating the parent directory. On a
clean checkout or after 'make clean', this fails with 'No such file or
directory', blocking 'make test' in environments with a global pg_worker.

This commit adds 'mkdir -p $(dirname $(PG_WORKER_PATH))' before the
install command in both branches of the conditional to ensure the target
directory always exists.

Also removes invalid 'noImportCycles' Biome rule that was causing
check-fmt to fail, and applies auto-formatting to test files.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@leynos leynos changed the title Implement backend revision-safe interests update strategy (3.5.4) Create destination directory before copying pg_worker Mar 16, 2026
codescene-delta-analysis[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 `@Makefile`:
- Around line 168-181: The prepare-pg-worker target duplicates mkdir -p
"$$(dirname "$(PG_WORKER_PATH)")" in both branches; move that directory creation
above the if to run unconditionally. Update the prepare-pg-worker recipe so it
first runs mkdir -p "$$(dirname "$(PG_WORKER_PATH)")" once, then performs the if
command -v pg_worker ... else ... install/cargo steps and the final install of
"$(PG_WORKER_INSTALL_ROOT)/bin/pg_worker" to "$(PG_WORKER_PATH)"; ensure you
keep the same shell escaping and variables (PG_WORKER_PATH,
PG_WORKER_INSTALL_ROOT, PG_EMBED_SETUP_UNPRIV_VERSION) intact.
🪄 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: e27851e7-0c66-4693-ac37-30b410f8fd17

📥 Commits

Reviewing files that changed from the base of the PR and between 7d9d770 and b10b81c.

📒 Files selected for processing (2)
  • Makefile
  • biome.jsonc
💤 Files with no reviewable changes (1)
  • biome.jsonc

Comment thread Makefile
…date

This commit addresses inline and outside-diff code review findings:

Code changes:
- Add Rustdoc examples to UserInterests::new and revision() methods
  demonstrating construction and revision assertion
- Fix test in users/tests.rs to assert on HTTP DTO (InterestsRequest)
  instead of domain DTO for proper boundary testing
- Add revision field assertion to user_interests_schema test
- Add skip_serializing_if to InterestsRequest.expected_revision to omit
  None values from JSON serialization
- Deduplicate mkdir command in prepare-pg-worker Makefile target by
  moving it before the conditional

Documentation changes:
- Update execplan, backend architecture, and PWA data model docs to
  correctly describe conflict response envelope shape: top-level
  code: "conflict" with nested details containing code:
  "revision_mismatch", expectedRevision, and actualRevision
- Fix first-person narration in toolchain errors investigation doc,
  replacing "I re-ran" with passive constructions
- Convert all headings to sentence case in toolchain errors doc per
  style guidelines
- Update PWA data model doc timestamp to current date

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos leynos changed the title Create destination directory before copying pg_worker Revision-safe interests; CI wiring; create pg_worker dir Mar 16, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (3)
docs/investigations/2026-03-13-toolchain-errors.md (1)

348-352: ⚠️ Potential issue | 🟡 Minor

Remove first-person phrasing in the diagnosis section.

Rewrite the heading at Line 348 to neutral technical voice to match repository documentation style.

♻️ Proposed fix
-What I cannot prove from current evidence:
+What cannot be proven from current evidence:

As per coding guidelines, "Avoid 2nd person or 1st person pronouns ("I", "you", "we")."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/investigations/2026-03-13-toolchain-errors.md` around lines 348 - 352,
The diagnosis heading "What I cannot prove from current evidence:" uses
first-person phrasing; change it to a neutral technical voice (e.g., "Unproven
causes based on current evidence" or "Potential root causes not proven by
current evidence") and scan the surrounding diagnosis section for any other
first- or second-person pronouns (e.g., "I", "we", "you") and replace them with
neutral phrasing so the entire diagnosis reads in objective, third-person
technical style.
backend/src/inbound/http/users/tests.rs (1)

359-359: 🧹 Nitpick | 🔵 Trivial

Rename the test to match the DTO under test.

Rename the function so it reflects InterestsRequest, not UpdateUserInterestsRequest.

♻️ Proposed fix
-fn update_user_interests_request_serializes_expected_revision_in_camel_case() {
+fn interests_request_serializes_expected_revision_in_camel_case() {
🤖 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` at line 359, The test function name
update_user_interests_request_serializes_expected_revision_in_camel_case is
misleading; rename it to reflect the DTO under test (InterestsRequest) — e.g.,
interests_request_serializes_expected_revision_in_camel_case — by updating the
function declaration and any references to that test so its name matches the
tested type (InterestsRequest) and keeps the same test body and attributes.
docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md (1)

205-207: ⚠️ Potential issue | 🟡 Minor

Correct the conflict-envelope wording in the decision log.

Align this entry with the implemented envelope: top-level code: "conflict" and nested details code: "revision_mismatch".

🛠️ Proposed fix
-- Decision: align stale-write error mapping with the existing preferences and
-  annotations conflict envelope using `code: revision_mismatch`,
-  `expectedRevision`, and `actualRevision`.
+- Decision: align stale-write error mapping with the existing preferences and
+  annotations conflict envelope using top-level `code: "conflict"` and nested
+  details containing `code: "revision_mismatch"`, `expectedRevision`, and
+  `actualRevision`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md`
around lines 205 - 207, The decision log currently states the stale-write error
mapping uses top-level `code: revision_mismatch`; update the wording to match
the implemented envelope: top-level `code: "conflict"` with nested details
containing `code: "revision_mismatch"` and the `expectedRevision` /
`actualRevision` fields; edit the sentence in the decision entry so it
explicitly describes the top-level `conflict` code and nested
`revision_mismatch` details (and keep the mention of `expectedRevision` and
`actualRevision` in the nested details).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md`:
- Around line 420-493: The ordered execution steps use repeated "1." markers;
replace each repeated "1." with ascending numbers (1., 2., 3., ...) so the steps
are sequential and deterministic, keeping the existing code blocks and examples
intact and ensuring any nested lists remain properly indented; search for the
repeated "1." list items (the top-level execution steps that contain the cargo
test and make commands and the "Expected ..." blocks) and renumber them in
order, and if your repository guideline requires first-level bullets be hyphens,
convert the very top-level list marker to "-" and then use sequential numeric
markers for the ordered sub-steps.

In `@docs/investigations/2026-03-13-toolchain-errors.md`:
- Around line 311-313: The markdown paragraph containing the sentence starting
with "The command `cargo clippy --workspace --all-targets --all-features -- -D
warnings`..." exceeds 80 columns and triggers MD013; reflow that sentence into
multiple lines so each line is ≤80 characters (for example split after the
clause "...-D warnings` was also re-run." and continue the rest on following
wrapped lines) ensuring the meaning stays unchanged and no backticks or inline
code are broken.

---

Duplicate comments:
In `@backend/src/inbound/http/users/tests.rs`:
- Line 359: The test function name
update_user_interests_request_serializes_expected_revision_in_camel_case is
misleading; rename it to reflect the DTO under test (InterestsRequest) — e.g.,
interests_request_serializes_expected_revision_in_camel_case — by updating the
function declaration and any references to that test so its name matches the
tested type (InterestsRequest) and keeps the same test body and attributes.

In `@docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md`:
- Around line 205-207: The decision log currently states the stale-write error
mapping uses top-level `code: revision_mismatch`; update the wording to match
the implemented envelope: top-level `code: "conflict"` with nested details
containing `code: "revision_mismatch"` and the `expectedRevision` /
`actualRevision` fields; edit the sentence in the decision entry so it
explicitly describes the top-level `conflict` code and nested
`revision_mismatch` details (and keep the mention of `expectedRevision` and
`actualRevision` in the nested details).

In `@docs/investigations/2026-03-13-toolchain-errors.md`:
- Around line 348-352: The diagnosis heading "What I cannot prove from current
evidence:" uses first-person phrasing; change it to a neutral technical voice
(e.g., "Unproven causes based on current evidence" or "Potential root causes not
proven by current evidence") and scan the surrounding diagnosis section for any
other first- or second-person pronouns (e.g., "I", "we", "you") and replace them
with neutral phrasing so the entire diagnosis reads in objective, third-person
technical style.
🪄 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: 4f67615a-a131-476c-b017-fd3264d3e8f1

📥 Commits

Reviewing files that changed from the base of the PR and between b10b81c and cd9af0b.

📒 Files selected for processing (9)
  • Makefile
  • backend/src/domain/user_interests.rs
  • backend/src/inbound/http/schemas.rs
  • backend/src/inbound/http/users.rs
  • backend/src/inbound/http/users/tests.rs
  • docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md
  • docs/investigations/2026-03-13-toolchain-errors.md
  • docs/wildside-backend-architecture.md
  • docs/wildside-pwa-data-model.md

Comment thread docs/investigations/2026-03-13-toolchain-errors.md Outdated
@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 16, 2026

@coderabbitai Please suggest a fix for this issue and supply a prompt for an AI coding agent to enable it to apply the fix:

backend/src/inbound/http/schemas.rs

Comment on file

        assert!(
            schema_json.contains("revision"),
            "schema should contain revision field"
        );

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: tests.error_schema_has_expected_name,tests.user_interests_schema_has_expected_name

Fix MD013/line-length violation at line 312 by wrapping the sentence to
stay within the 80-character limit.

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

This comment was marked as resolved.

@leynos
Copy link
Copy Markdown
Owner Author

leynos commented Mar 16, 2026

@coderabbitai have the following now been resolved?

docs/investigations/2026-03-13-toolchain-errors.md (1)

348-352: ⚠️ Potential issue | 🟡 Minor
Remove first-person phrasing in the diagnosis section.
Rewrite the heading at Line 348 to neutral technical voice to match repository documentation style.

As per coding guidelines, "Avoid 2nd person or 1st person pronouns ("I", "you", "we")."

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

359-359: 🧹 Nitpick | 🔵 Trivial
Rename the test to match the DTO under test.
Rename the function so it reflects InterestsRequest, not UpdateUserInterestsRequest.

docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md (1)

205-207: ⚠️ Potential issue | 🟡 Minor
Correct the conflict-envelope wording in the decision log.
Align this entry with the implemented envelope: top-level code: "conflict" and nested details code: "revision_mismatch".

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Mar 16, 2026

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== docs/investigations/2026-03-13-toolchain-errors.md lines 340-360 ==="
sed -n '340,360p' docs/investigations/2026-03-13-toolchain-errors.md

echo ""
echo "=== backend/src/inbound/http/users/tests.rs lines 350-370 ==="
sed -n '350,370p' backend/src/inbound/http/users/tests.rs

echo ""
echo "=== docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md lines 198-215 ==="
sed -n '198,215p' docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md

Length of output: 2687


@leynos: None of the three are resolved yet. Here is the current state of each:


1. docs/investigations/2026-03-13-toolchain-errors.md — first-person phrasing

Still present (line ~349):

What I cannot prove from current evidence:

Rewrite to neutral voice, e.g.:

What the available evidence does not establish:

2. backend/src/inbound/http/users/tests.rs — test name reflects wrong DTO

Still named (line 359):

fn update_user_interests_request_serializes_expected_revision_in_camel_case()

The test body constructs an InterestsRequest, not an UpdateUserInterestsRequest. Rename to:

fn interests_request_serializes_expected_revision_in_camel_case()

3. docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md — conflict-envelope wording

Current text (~lines 205–207):

align stale-write error mapping with the existing preferences and
annotations conflict envelope using `code: revision_mismatch`,
`expectedRevision`, and `actualRevision`.

This omits the two-level envelope. Update to reflect the implemented shape — top-level code: "conflict" wrapping a details object with code: "revision_mismatch", e.g.:

align stale-write error mapping with the existing preferences and
annotations conflict envelope: top-level `code: "conflict"` with nested
details carrying `code: "revision_mismatch"`, `expectedRevision`, and
`actualRevision`.

leynos and others added 2 commits March 16, 2026 23:07
Replace repeated "1." markers with sequential numbering (1., 2., 3., 4.,
5., 6.) in the concrete execution steps section. Indent all list item
content (code blocks and text) to maintain proper list continuity and
satisfy MD029/ol-prefix markdownlint rule with "ordered" style.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Introduce assert_schema_contains helper to replace repeated pattern across
all five has_expected_name test functions. The helper takes the expected
schema name and a slice of field names to check, eliminating ~40 lines of
duplicated test code.

Also remove unused UpdateUserInterestsRequest import from users/tests.rs
that was left over from the previous refactoring.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
@leynos leynos changed the title Revision-safe interests; CI wiring; create pg_worker dir Revision-safe interests; CI wiring; remove pg_worker; create dest dir Mar 16, 2026
codescene-delta-analysis[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

♻️ Duplicate comments (1)
docs/investigations/2026-03-13-toolchain-errors.md (1)

348-348: ⚠️ Potential issue | 🟡 Minor

Eliminate first-person phrasing in the heading text.

Replace What I cannot prove from current evidence: with neutral wording, e.g.
What cannot be proved from current evidence:.

As per coding guidelines: "Avoid 2nd person or 1st person pronouns ('I', 'you', 'we')".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/investigations/2026-03-13-toolchain-errors.md` at line 348, Change the
heading text to remove first-person phrasing: replace the line "What I cannot
prove from current evidence:" with a neutral form such as "What cannot be proved
from current evidence:" so it conforms to the guideline to avoid
first‑person/second‑person pronouns.
🤖 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 358-369: Rename the test function to reference the DTO (e.g.,
update_user_interests_request_serializes_expected_revision_in_camel_case ->
interests_request_serializes_expected_revision_in_camel_case) and harden it by
also asserting that the snake_case key "expected_revision" is not present in the
serialized JSON; locate the InterestsRequest creation and serde_json::to_value
call in the test and add an assertion that value.get("expected_revision") is
None while keeping the existing assertion that
value.get("expectedRevision").and_then(Value::as_u64) == Some(3).

In `@docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md`:
- Line 228: The phrase "an existing preferences row" has incorrect article/noun
agreement; locate that exact phrase in the document (search for "an existing
preferences row") and replace it with either "an existing preference row" for
singular or "existing preferences rows" for plural depending on the intended
meaning; ensure surrounding sentence grammar remains correct after the
substitution.

In `@docs/investigations/2026-03-13-toolchain-errors.md`:
- Around line 497-500: The long bullet lines under the "prepare-pg-worker `os
error 2`" section exceed 80 columns; split each long command bullet into a short
descriptive bullet (label) and a following fenced code block so lines wrap under
80 columns; specifically update the bullet containing the strace command
(`strace -f -o /tmp/pg-worker-build.strace cargo build -p backend --bin
pg_worker`) into a label like "Capture strace output:" followed by a fenced code
block with the command, and ensure the other bullets referencing `ls -l`, `ldd`,
`mount`, and `df` are similarly wrapped and line-wrapped to respect the
80-column Markdown rule.

---

Duplicate comments:
In `@docs/investigations/2026-03-13-toolchain-errors.md`:
- Line 348: Change the heading text to remove first-person phrasing: replace the
line "What I cannot prove from current evidence:" with a neutral form such as
"What cannot be proved from current evidence:" so it conforms to the guideline
to avoid first‑person/second‑person pronouns.
🪄 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: be5ba56c-940d-4a70-808d-9a28e10d3dcd

📥 Commits

Reviewing files that changed from the base of the PR and between cd9af0b and 829b4fb.

📒 Files selected for processing (4)
  • backend/src/inbound/http/schemas.rs
  • backend/src/inbound/http/users/tests.rs
  • docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md
  • docs/investigations/2026-03-13-toolchain-errors.md

Comment thread backend/src/inbound/http/users/tests.rs
Comment thread docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md Outdated
Comment thread docs/investigations/2026-03-13-toolchain-errors.md
leynos and others added 2 commits March 17, 2026 13:00
Fix three remaining issues from code review:

1. Replace first-person phrasing "What I cannot prove" with neutral
   voice "What the available evidence does not establish" in toolchain
   errors investigation doc

2. Rename test function from
   update_user_interests_request_serializes_expected_revision_in_camel_case
   to interests_request_serializes_expected_revision_in_camel_case to
   match the actual DTO being tested (InterestsRequest)

3. Update execplan conflict-envelope description to accurately reflect
   the two-level structure: top-level code: "conflict" with nested
   details containing code: "revision_mismatch", expectedRevision, and
   actualRevision

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Fix three remaining issues from code review:

1. Harden interests_request serialization test by adding assertion that
   snake_case key "expected_revision" is not present in JSON, ensuring
   only camelCase "expectedRevision" is serialized

2. Fix article/noun agreement in execplan: change "an existing
   preferences row" to "an existing preference row" (singular)

3. Update toolchain investigation doc to use neutral voice: change "What
   the available evidence does not establish:" to "What cannot be proved
   from current evidence:" to avoid first-person phrasing

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (1)
docs/investigations/2026-03-13-toolchain-errors.md (1)

497-500: ⚠️ Potential issue | 🟡 Minor

Reflow the long bullet lines to satisfy the 80-column rule.

Break the long command bullets at Line 498 and Line 499 into short labels plus
fenced blocks so every bullet line stays within 80 columns.

Proposed fix
 - If the `prepare-pg-worker` `os error 2` recurs, capture:
-  - `strace -f -o /tmp/pg-worker-build.strace cargo build -p backend --bin pg_worker`
-  - `ls -l` and `ldd` for the exact `rustc` path named in the failure
+  - strace output:
+
+    ```text
+    strace -f -o /tmp/pg-worker-build.strace cargo build -p backend --bin pg_worker
+    ```
+
+  - `ls -l` and `ldd` output for the exact `rustc` path named in the
+    failure
   - `mount`, `df`, and any container lifecycle logs

As per coding guidelines: "Markdown paragraphs and bullet points must be wrapped
at 80 columns."

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@docs/investigations/2026-03-13-toolchain-errors.md` around lines 497 - 500,
Reflow the long bullet lines under the "prepare-pg-worker `os error 2`" note so
each markdown paragraph and list item is wrapped to 80 columns: break the long
`strace -f -o /tmp/pg-worker-build.strace cargo build -p backend --bin
pg_worker` line into a short bullet label (e.g., "Capture a strace of the
build:") followed by a fenced code block containing the full strace command, do
the same for the `ls -l` and `ldd` instruction (e.g., "Collect `ls -l` and `ldd`
output for the `rustc` path:" with a fenced block for commands), and ensure the
final bullet for `mount`, `df`, and container lifecycle logs is wrapped to 80
columns by splitting or rephrasing into multiple wrapped lines; keep the content
identical but formatted to meet the 80-column rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@docs/investigations/2026-03-13-toolchain-errors.md`:
- Around line 497-500: Reflow the long bullet lines under the "prepare-pg-worker
`os error 2`" note so each markdown paragraph and list item is wrapped to 80
columns: break the long `strace -f -o /tmp/pg-worker-build.strace cargo build -p
backend --bin pg_worker` line into a short bullet label (e.g., "Capture a strace
of the build:") followed by a fenced code block containing the full strace
command, do the same for the `ls -l` and `ldd` instruction (e.g., "Collect `ls
-l` and `ldd` output for the `rustc` path:" with a fenced block for commands),
and ensure the final bullet for `mount`, `df`, and container lifecycle logs is
wrapped to 80 columns by splitting or rephrasing into multiple wrapped lines;
keep the content identical but formatted to meet the 80-column rule.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: be07cad3-8930-48ba-aaac-3dec07ff1097

📥 Commits

Reviewing files that changed from the base of the PR and between 829b4fb and 1fcdadd.

📒 Files selected for processing (3)
  • backend/src/inbound/http/users/tests.rs
  • docs/execplans/backend-3-5-4-revision-safe-interests-update-strategy.md
  • docs/investigations/2026-03-13-toolchain-errors.md

@leynos leynos merged commit b810523 into main Mar 18, 2026
4 checks passed
@leynos leynos deleted the implement-revision-safe-update-strategy-i43x3p branch March 18, 2026 12:54
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