Skip to content

Plan REST surface hardening (4.1.2) - #111

Merged
leynos merged 24 commits into
mainfrom
4-1-2-finalize-rest-surfaces
May 31, 2026
Merged

Plan REST surface hardening (4.1.2)#111
leynos merged 24 commits into
mainfrom
4-1-2-finalize-rest-surfaces

Conversation

@leynos

@leynos leynos commented May 24, 2026

Copy link
Copy Markdown
Owner

Summary

This branch carries the pre-implementation ExecPlan for roadmap item
4.1.2. The plan covers eight milestones that finalize REST surfaces for
the previously implemented /v1 canonical resources: a unified
{code, message, details} error envelope served through a single Falcon
add_error_handler, the {items, limit, offset, total} pagination
envelope across every list endpoint, consistent filter parameter
parsing, and an inbound-adapter-local AuthorizationPort scaffold with
a permit-all default. Full Role-Based Access Control (RBAC) and tenancy
isolation remain roadmap item 5.1.

No production code changes ship in this branch. The plan authorises the
work that follows after explicit user approval.

Review walkthrough

  • Start with the Purpose and big picture section for the eight observable-success criteria.
  • Read Constraints, Tolerances, and Risks to confirm the hexagonal-architecture rails and scope caps fit the work.
  • Review the Decision log — six explicit decisions on total plumbing, error-envelope handler placement, the authorization scaffold location, resolved-bindings pagination, the readiness 503 exemption, and the deliberate deferral of Idempotency-Key/Retry-After.
  • Then walk the Plan of work milestone-by-milestone (error envelope → reference-domain total → pagination retrofit → history pagination → filter consistency → authorization scaffold → docs → gates).
  • Finish with Validation and acceptance and Interfaces and dependencies to see the gate set and the planned module surface.

Notes

  • The plan adopts the project's documented error envelope
    ({code, message, details}) from
    docs/episodic-tui-api-design.md
    rather than RFC 9457 Problem Details verbatim, so the response
    Content-Type remains application/json.
  • Three Wyvern research subagents (API surface inventory, test coverage
    survey, hexagonal/architecture guard) contributed the file:line
    evidence that underpins the constraints, risks, and milestone
    scoping.
  • Implementation does not begin until the plan is approved.

References

Summary by Sourcery

Documentation:

  • Document a detailed multi-milestone execution plan for finalizing REST surfaces on existing /v1 endpoints, including error and pagination contracts, filter consistency, and an authorization scaffold.

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Summary

This pull request implements roadmap item 4.1.2 ("Finalize REST surfaces for previous phase artefacts"), marking the work complete via a new ExecPlan document (docs/execplans/4-1-2-finalize-rest-surfaces.md). The implementation harddens all /v1 REST canonical endpoints through four cross-cutting concerns:

1. Unified Error Envelope

A new error serialisation layer (episodic/api/errors.py) replaces the previous async handler with a synchronous serialize_http_error() function that standardises all 4xx/5xx responses to a stable JSON envelope containing {code, message, details}. This is integrated via app.set_error_serializer(serialize_http_error) in episodic/api/app.py. Helper functions map_profile_template_error() and map_reference_error() translate domain exceptions into enriched Falcon errors with envelope metadata. Validation errors surface structured details containing field and constraint metadata. The 404 response for unregistered routes now returns the canonical envelope shape rather than Falcon's default (docs/developers-guide.md updated).

2. Pagination Envelopes & total Plumbing

All list endpoints now return a consistent pagination envelope: {items, limit, offset, total}. A new Pagination value object (episodic/canonical/pagination.py) encapsulates request-level page boundaries. New paged service functions across profile templates, reference documents, revisions, and bindings (e.g., list_entities_with_revisions_paged, list_history_paged, list_reference_documents_paged) compute the unpaginated total by separate count queries on matching repository methods. Repository protocols and implementations now include .list(..., limit, offset) and .count(...) variants:

  • Profile/template repositories (episodic/canonical/profile_templates/types.py, episodic/canonical/storage/repositories.py): _SeriesProfileRepository.list() and _EpisodeTemplateRepository.list() now accept limit and offset; new .count() methods return totals.
  • History repositories (episodic/canonical/history_protocols.py, episodic/canonical/storage/history_repositories.py): list_for_profile_paged(), list_for_template_paged(), and corresponding count methods replace non-paged variants.
  • Reference repositories (episodic/canonical/reference_protocols.py, episodic/canonical/storage/reference_repositories.py): New count_for_series(), count_for_document(), and count_for_target() methods support pagination across documents, revisions, and bindings.

All REST resources (episodic/api/resources/*.py) updated to use paged endpoints and return the full pagination envelope. History endpoints (/v1/.../history) now support pagination query parameters and return paginated responses via HistoryRequest wrapper passed to handle_get_history().

3. Authorisation Scaffold

A new AuthorizationPort protocol and minimal Falcon middleware (episodic/api/authorization.py) implement a permit-all default adapter. The contract defines AuthorizationDecision outcomes (PERMIT, UNAUTHORIZED, FORBIDDEN) and an async decide(context) method operating on paths starting with /v1/. Non-permit decisions short-circuit with appropriate HTTP status codes (401/403) and serialised error envelopes; adapter failures return HTTP 503. The scaffold is integrated into app initialisation (episodic/api/app.py) and dependency injection (ApiDependencies in episodic/api/dependencies.py). Full RBAC and tenancy isolation are deferred to roadmap item 5.1 (docs/developers-guide.md, docs/users-guide.md updated).

4. Consistent Filter Parsing

New helper functions (parse_optional_uuid_param, parse_enum_param in episodic/api/helpers.py) standardise optional query-parameter validation. The parse_pagination helper now returns a typed Pagination object instead of a tuple. Reference-binding and reference-document list endpoints now validate and normalise enum filters (e.g., kind, target_kind) before dispatching to service logic, surfacing validation failures via structured error envelopes.

Supporting Changes

  • Documentation: docs/developers-guide.md, docs/users-guide.md, and docs/episodic-podcast-generation-system-design.md updated to document the /v1 error, pagination, filter, and authorisation contracts.
  • Roadmap: docs/roadmap.md marks 4.1.2 as complete.
  • Tests: Comprehensive test coverage added for error envelopes (tests/test_api_error_envelope.py), authorisation behaviour (tests/test_api_authorization.py), pagination envelopes across profile/template and reference endpoints (tests/test_profile_template_pagination_api.py, tests/test_profile_template_history_pagination_api.py, tests/test_reference_document_roundtrip.py), and route versioning (tests/test_api_route_versioning.py). Existing tests refactored for parametrisation and clarity; test fixtures extended with helper builders (build_api_dependencies, CanonicalApiCreators).
  • Code quality: Inlined duplicated builders in episodic/canonical/profile_templates/services/_generic.py into _get_repos_for_kind; parametrised error-envelope tests; extracted helper functions in long test functions; applied per-file linter suppressions (pyproject.toml).

Related issues: [#125](https://github.com/.../ issues/125) (pyscn clone detection trial). References: ADR 009 (TUI API design), Lody session recordings, and Wyvern research subagents (as documented in the PR objectives).

Walkthrough

Add a central JSON error envelope and serializer, introduce an /v1 authorisation port and Falcon middleware, implement paginated list/history APIs with count methods across services and repositories, wire Falcon app to use the serializer and middleware, update helpers/handlers/resources to return pagination envelopes, and add tests and docs.

Changes

Project Preparation and Roadmap

Layer / File(s) Summary
Local development environment ignore patterns
.gitignore
Add .claude/ to .gitignore and remove obsolete .repo.lock ignore entry.
Roadmap and ExecPlan
docs/execplans/4-1-2-finalize-rest-surfaces.md, docs/roadmap.md, docs/developers-guide.md, docs/users-guide.md, docs/episodic-podcast-generation-system-design.md
Add ExecPlan for 4.1.2, mark task complete, and update user/developer/system-design docs to describe pagination envelopes, unified error contract ({code,message,details}), filter validation, and /v1 authorisation scaffold.

API Infrastructure and Behaviour

Layer / File(s) Summary
Central error envelope and mappers
episodic/api/errors.py
Add frozen ErrorEnvelope, serialize_http_error, http_error, validation_error, and domain-to-HTTP mappers (map_profile_template_error, map_reference_error) to produce canonical {code,message,details} payloads.
Authorisation scaffold and middleware
episodic/api/authorization.py, episodic/api/dependencies.py
Introduce AuthorizationDecision, AuthorizationContext, AuthorizationPort protocol, PermitAll adapter, AuthorizationMiddleware (applies to /v1/ only), extend ApiDependencies.authorization with validation.
Falcon app wiring
episodic/api/app.py, episodic/api/__init__.py
Wire AuthorizationMiddleware into app creation and register serialize_http_error as the app-wide Falcon error serialiser; re-export auth types from package root.

Request parsing, handlers and resources

Layer / File(s) Summary
Request helpers and validators
episodic/api/helpers.py
Replace direct Falcon exceptions with validation_error, add parse_pagination (returns Pagination), parse_optional_uuid_param, parse_enum_param, and stricter field/constraint metadata for validation errors.
Handlers and history pagination
episodic/api/handlers.py, episodic/api/resources/base.py
Introduce HistoryRequest[EntityT], change handle_get_history to accept Pagination and expect (items, total) from services; centralise domain-error mapping via map_profile_template_error.
Resource endpoints
episodic/api/resources/*
Update series-profiles, episode-templates, reference-documents, reference-bindings, resolved-bindings resources to parse pagination and filters, call paged service variants, and include items, limit, offset, and total in responses.

Canonical services and storage

Layer / File(s) Summary
Pagination value and services
episodic/canonical/pagination.py, episodic/canonical/profile_templates/services/_generic.py, episodic/canonical/profile_templates/*
Add Pagination type; add list_history_paged and list_entities_with_revisions_paged, extend kind-dispatch to support paged list/count helpers returning (items,total).
Repository protocols and storage implementations
episodic/canonical/*/types.py, episodic/canonical/reference_protocols.py, episodic/canonical/storage/*
Add paged list and count methods to protocols; implement SQLAlchemy paged queries and explicit count queries; add _document_series_filter helper and reference repo count methods.
Reference-domain helpers
episodic/canonical/reference_documents/*
Add list_reference_*_paged helpers that validate pagination, call storage list and count functions, and return (items,total); keep non-paged functions delegating to paged helpers.

Tests and fixtures

Layer / File(s) Summary
Test fixtures
tests/fixtures/api.py
Add build_api_dependencies to inject authorization test adapters; update fixtures to use it.
Authorisation and envelope tests
tests/test_api_authorization.py, tests/test_api_error_envelope.py
Add tests for authorisation outcomes (200/401/403/503) and strict error-envelope assertions (code, message, details).
Pagination and integration tests
tests/*_pagination_*.py, tests/test_reference_document_roundtrip.py, updated tests
Add parametrised tests for list/history pagination envelopes and totals; update existing tests to assert pagination fields and canonical error envelopes; add helpers asserting total >= returned items.

Sequence Diagram(s)

(omit — changes are infra, contracts and multiple small interactions; no multi-component sequential diagram generated)

Possibly related PRs

  • leynos/episodic#105: Related /v1 routing and middleware work that intersects with AuthorizationMiddleware and app wiring in this PR.
You are an AI agent. Produce a compact reviewer checklist for PR 111 summarising the major checkpoints: docs review, API wiring, auth middleware, error-envelope logic, pagination service changes, storage queries, and test updates.

Reviewer checklist (compact)

  • Read ExecPlan and docs changes; confirm samples and roadmap marker.
  • Verify .gitignore change only affects listed pattern.
  • Inspect episodic/api/errors.py: validate ErrorEnvelope, serialize_http_error mapping, and domain->HTTP mappers.
  • Inspect episodic/api/authorization.py and episodic/api/dependencies.py: validate AuthorizationPort API, PermitAll, middleware path gating, denial logging, and dependency validation.
  • Inspect episodic/api/app.py and init.py: confirm middleware installed and serializer wired; confirm re-exports.
  • Review episodic/api/helpers.py: confirm parse_pagination returns Pagination and validation_error usage for field/constraint metadata.
  • Review episodic/api/handlers.py and resources/*: confirm handlers accept/propagate Pagination, HistoryRequest wiring, resource responses include limit/offset/total.
  • Review episodic/canonical/pagination.py and profile_templates services: confirm list_history_paged and list_entities_with_revisions_paged signatures and behaviour.
  • Review protocol/type changes: confirm new count/list_paged methods in reference and history protocols/types.
  • Review SQLAlchemy storage changes: confirm paged queries, count implementations, and _document_series_filter correctness.
  • Run or inspect tests/fixtures changes: confirm build_api_dependencies injection, canonical_api_creators fixture.
  • Run test suite (or inspect tests): ensure new tests for auth, error envelopes, and pagination pass and updated assertions match envelope shape and total semantics.
  • Spot-check logging and exception handling paths: adapter exception -> 503 envelope; unexpected decision -> 503 internal_error envelope.
  • Confirm no exported public API regressions beyond documented new exports.
  • Confirm pyproject.toml and lint config changes are intentional.

"Ignore the noise, make contracts bright,
Errors shaped and pages sliced just right,
Authorise /v1 with a scaffold light,
Tests assert totals, docs mark the flight."

✨ 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 4-1-2-finalize-rest-surfaces

@sourcery-ai

sourcery-ai Bot commented May 24, 2026

Copy link
Copy Markdown

Reviewer's Guide

Adds a comprehensive execution plan document for roadmap item 4.1.2 that specifies how to harden existing /v1 REST endpoints with a unified error envelope, consistent pagination (including total counts), filter handling, and an authorization scaffold, without changing any production behavior yet.

Sequence diagram for authorization scaffold and unified error envelope

sequenceDiagram
    actor Client
    participant FalconApp
    participant AuthorizationMiddleware
    participant Resource
    participant AuthorizationPort
    participant ErrorHandler as handle_http_error

    Client->>FalconApp: HTTP request /v1/series-profiles
    FalconApp->>AuthorizationMiddleware: process_request(req, resp)
    AuthorizationMiddleware->>AuthorizationPort: decide(AuthorizationContext)
    AuthorizationPort-->>AuthorizationMiddleware: AuthorizationDecision.permit
    AuthorizationMiddleware-->>FalconApp: continue
    FalconApp->>Resource: on_get(req, resp)
    alt [domain or validation error]
        Resource->>FalconApp: raise falcon.HTTPError
        FalconApp->>ErrorHandler: handle_http_error(req, resp, exc, params)
        ErrorHandler-->>Client: 4xx/5xx ErrorEnvelope
    else [success]
        Resource-->>Client: 2xx response (possibly pagination envelope)
    end
Loading

Flow diagram for paginated list endpoints with total counts

flowchart LR
    Client[Client] --> ApiResource[API Resource on_get]
    ApiResource --> Helpers[parse_pagination]
    Helpers --> Service[list_reference_documents_paged]
    Service --> RepoList[ReferenceDocumentRepository.list_for_series]
    Service --> RepoCount[ReferenceDocumentRepository.count_for_series]
    RepoList --> DB[(Database)]
    RepoCount --> DB
    Service --> ApiResource
    ApiResource --> Response["Pagination envelope {items, limit, offset, total}"]
Loading

File-Level Changes

Change Details Files
Introduce an ExecPlan documenting the REST surface hardening work for /v1 endpoints (errors, pagination, filters, authorization scaffold) and its constraints, risks, and milestones.
  • Describe purpose, scope, and success criteria for finalizing REST surfaces on existing canonical resources without implementing code changes yet.
  • Define architectural and behavioral constraints, tolerances, and risks around adding a unified error envelope, pagination total counts, and authorization seams.
  • Lay out milestone-by-milestone implementation steps for central error handling, pagination plumbing, filter normalization, authorization middleware, documentation updates, and quality gates.
  • Record decisions about repository count_* methods, Falcon error handler structure, authorization port placement, resolved-bindings pagination strategy, readiness 503 exemption, and deferring idempotency/rate-limiting work.
  • Specify validation and acceptance criteria plus interfaces and dependencies for upcoming changes, including new API modules and repository/service additions.
docs/execplans/4-1-2-finalize-rest-surfaces.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

codescene-delta-analysis[bot]

This comment was marked as outdated.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@lodyai
lodyai Bot force-pushed the 4-1-2-finalize-rest-surfaces branch from bb5ef38 to 348b380 Compare May 25, 2026 01:20
codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos
leynos marked this pull request as ready for review May 25, 2026 19:37

@sourcery-ai sourcery-ai Bot left a comment

Copy link
Copy Markdown

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

@coderabbitai coderabbitai Bot added the Roadmap label May 25, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/execplans/4-1-2-finalize-rest-surfaces.md`:
- Line 149: The document uses mixed -ise/-yse spellings; normalize to
en-GB-oxendict forms using -ize and -lyse (e.g., replace "stabilise" with
"stabilize", "centralised" → "centralized", "serialisation" → "serialization",
"organisations" → "organizations", "optimisation" → "optimization", "organised"
→ "organized" and any -lyse variants to -lyse), and apply the same replacements
in the other flagged locations (lines referenced: 230, 338, 347, 357, 549) so
the whole file consistently follows the mandated -ize/-lyse suffix convention.
🪄 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 Plus

Run ID: 493a352b-0423-434b-b3dd-29f9280efd76

📥 Commits

Reviewing files that changed from the base of the PR and between 3403ace and 348b380.

📒 Files selected for processing (2)
  • .gitignore
  • docs/execplans/4-1-2-finalize-rest-surfaces.md

Comment thread docs/execplans/4-1-2-finalize-rest-surfaces.md Outdated
codescene-delta-analysis[bot]

This comment was marked as outdated.

leynos added 3 commits May 30, 2026 02:38
Draft the pre-implementation ExecPlan for roadmap item `4.1.2` at
`docs/execplans/4-1-2-finalize-rest-surfaces.md`. The plan covers eight
milestones that finalize REST surfaces for previously implemented `/v1`
canonical resources: a unified `{code, message, details}` error envelope
served through a single Falcon `add_error_handler`, the
`{items, limit, offset, total}` pagination envelope across every list
endpoint, consistent filter parameter parsing, and an
inbound-adapter-local `AuthorizationPort` scaffold with a permit-all
default. Full Role-Based Access Control (RBAC) and tenancy isolation
remain roadmap item `5.1`.

Decisions are recorded in the plan's `Decision log`: additive `count_*`
Protocol methods (so existing list signatures stay backwards-compatible),
Pattern B (centralised Falcon error handler) plus per-family
classification helpers, scaffold port placed at
`episodic/api/authorization.py`, resolved-bindings paginated at the API
layer, and routine readiness `503` responses exempt from the envelope
rewrite. References to `docs/episodic-tui-api-design.md`,
`docs/episodic-podcast-generation-system-design.md`, and ADRs 002, 009,
and 014 anchor the constraints. Three Wyvern research subagents
contributed the API surface inventory, the test coverage map, and the
hexagonal/architecture rails check that underpin the plan.

Implementation is gated on user approval.
The Claude Code harness writes a per-session lock file at
`.claude/scheduled_tasks.lock` (sessionId, pid, procStart, acquiredAt).
This is local tooling state, not source, and mirrors the existing
treatment of `.agents/mcp/context_pack/packs/.repo.lock` already
ignored at line 10. Drop the duplicated `.agents/...` entry while
adding `.claude/`.
Update `docs/execplans/4-1-2-finalize-rest-surfaces.md` after the rebase
onto `origin/main` picked up commit `3403ace` ("Adopt Hecate for
architecture checks (#107)"), which removed the repo-local
`episodic.architecture` module and moved policy enforcement into
`[tool.hecate]` in `pyproject.toml:437-496`.

- Replace `episodic/architecture/policy.py:<line>` citations with
  `pyproject.toml:<line>` citations covering the same group-prefix and
  allowed-import rules.
- Replace the `python -m episodic.architecture` recovery instruction
  with `uv run hecate check`, noting that diagnostic identifier
  `ARCH001` carries over.
- Append a revision note documenting the rebase and confirming the
  policy semantics (group prefixes, allowed imports) are
  byte-equivalent to the previous Python-module implementation, so no
  work-plan milestone change was required.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Caution

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

⚠️ Outside diff range comments (4)
episodic/canonical/profile_templates/services/_generic.py (1)

65-181: 🛠️ Refactor suggestion | 🟠 Major | 🏗️ Heavy lift

Split _get_repos_for_kind into per-kind builders and shrink the method.

Reduce the branching and nested local function load in Line 65 onward by extracting each kind into dedicated helper factories, then keep _get_repos_for_kind as a thin dispatcher. The current shape already trips the pipeline’s large-method gate and will keep degrading maintainability as new kinds are added.

As per coding guidelines "Keep C90 / mccabe complexity ≤ 9".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/profile_templates/services/_generic.py` around lines 65 -
181, The function _get_repos_for_kind is too large and branches per EntityKind
(EntityKind.SERIES_PROFILE / "series_profile" and EntityKind.EPISODE_TEMPLATE /
"episode_template") with many nested local async functions; extract each branch
into its own helper factory (e.g., _build_series_profile_kind(uow:
CanonicalUnitOfWork) -> _KindDispatch and _build_episode_template_kind(uow:
CanonicalUnitOfWork) -> _KindDispatch) that move the repository casts, the
nested async functions (_list_profiles, _count_profiles,
_list_profile_history_paged, _list_templates, _list_template_history_paged) and
the _KindDispatch construction into the helper; then make _get_repos_for_kind a
thin dispatcher that calls the appropriate helper based on kind and returns its
_KindDispatch. Ensure signatures and returned callable types (entity_get,
fetch_latest, list_history_for_parent, list_history_for_parent_paged,
count_history_for_parent, list_entities, count_entities, get_latest_revisions)
match the originals.
tests/test_api_error_envelope.py (1)

46-166: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Parametrize the duplicated validation/envelope tests

Remove the repeated arrange/act/assert bodies and replace them with a single @pytest.mark.parametrize matrix covering all 7 cases in tests/test_api_error_envelope.py.

Suggested refactor shape
+@pytest.mark.parametrize(
+    ("path", "params", "status_code", "code", "field", "constraint"),
+    [
+        (
+            "/v1/series-profiles/not-a-valid-uuid",
+            None,
+            400,
+            "validation_error",
+            "profile_id",
+            "uuid",
+        ),
+        (
+            "/v1/reference-bindings",
+            {"target_kind": "episode_template", "target_id": "irrelevant", "limit": "0"},
+            400,
+            "validation_error",
+            "limit",
+            "range",
+        ),
+        (
+            "/v1/episode-templates",
+            {"series_profile_id": "not-a-uuid"},
+            400,
+            "validation_error",
+            "series_profile_id",
+            "uuid",
+        ),
+        (
+            "/v1/series-profiles/018f0c2a-1234-7000-a000-000000000001/reference-documents",
+            {"kind": "not-a-kind"},
+            400,
+            "validation_error",
+            "kind",
+            "enum",
+        ),
+        (
+            "/v1/reference-bindings",
+            {"target_kind": "not-a-target", "target_id": "018f0c2a-1234-7000-a000-000000000001"},
+            400,
+            "validation_error",
+            "target_kind",
+            "enum",
+        ),
+        (
+            "/v1/series-profiles/018f0c2a-1234-7000-a000-000000000001/resolved-bindings",
+            None,
+            400,
+            "validation_error",
+            "episode_id",
+            "required",
+        ),
+        (
+            "/v1/series-profiles/018f0c2a-1234-7000-a000-000000000001",
+            None,
+            404,
+            "not_found",
+            None,
+            None,
+        ),
+    ],
+)
+def test_error_envelope_matrix(
+    canonical_api_client: testing.TestClient,
+    path: str,
+    params: dict[str, str] | None,
+    status_code: int,
+    code: str,
+    field: str | None,
+    constraint: str | None,
+) -> None:
+    simulate_get_kwargs: dict[str, object] = {}
+    if params is not None:
+        simulate_get_kwargs["params"] = params
+
+    response = canonical_api_client.simulate_get(path, **simulate_get_kwargs)
+
+    _assert_error_envelope(
+        response,
+        status_code=status_code,
+        code=code,
+        field=field,
+        constraint=constraint,
+    )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_api_error_envelope.py` around lines 46 - 166, Multiple tests in
tests/test_api_error_envelope.py repeat the same arrange/act/assert pattern;
consolidate them by replacing the seven individual test_... functions with a
single parametrized test using pytest.mark.parametrize that iterates over cases
(use identifiers and expected attrs for:
test_invalid_uuid_returns_validation_envelope,
test_invalid_pagination_returns_validation_envelope,
test_invalid_optional_uuid_filter_returns_validation_envelope,
test_invalid_reference_document_kind_filter_returns_validation_envelope,
test_invalid_reference_binding_target_kind_returns_validation_envelope,
test_missing_query_parameter_returns_validation_envelope,
test_unknown_identifier_returns_not_found_envelope), call
canonical_api_client.simulate_get per case, and assert via
_assert_error_envelope using the tuple fields (path, params, expected
status_code, code, optional field and constraint) so each scenario is covered
without duplicated bodies.
episodic/api/handlers.py (1)

150-224: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Split handle_update_entity into focused helpers to clear the quality gate.

Extract required-field validation and error remapping from this method. Keep the method as request orchestration only so the CodeScene large-method gate stops failing.

♻️ Proposed refactor
+def _validate_required_fields(
+    payload: JsonPayload,
+    required_fields: tuple[str, ...],
+) -> None:
+    for field_name in required_fields:
+        if field_name not in payload:
+            msg = f"Missing required field: {field_name}"
+            raise validation_error(msg, field=field_name, constraint="required")
+
+
+def _raise_mapped_update_error(
+    exc: EntityNotFoundError | RevisionConflictError,
+    *,
+    parsed_entity_id: uuid.UUID,
+    payload: JsonPayload,
+) -> None:
+    if isinstance(exc, EntityNotFoundError):
+        raise map_profile_template_error(exc, entity_id=parsed_entity_id) from exc
+    expected_revision = typ.cast("int | None", payload.get("expected_revision"))
+    raise map_profile_template_error(
+        exc,
+        entity_id=parsed_entity_id,
+        expected_revision=expected_revision,
+    ) from exc
@@
-    for field_name in required_fields:
-        if field_name not in payload:
-            msg = f"Missing required field: {field_name}"
-            raise validation_error(msg, field=field_name, constraint="required")
+    _validate_required_fields(payload, required_fields)
@@
-    except EntityNotFoundError as exc:
-        raise map_profile_template_error(exc, entity_id=parsed_entity_id) from exc
-    except RevisionConflictError as exc:
-        expected_revision = typ.cast("int | None", payload.get("expected_revision"))
-        raise map_profile_template_error(
-            exc,
-            entity_id=parsed_entity_id,
-            expected_revision=expected_revision,
-        ) from exc
+    except (EntityNotFoundError, RevisionConflictError) as exc:
+        _raise_mapped_update_error(
+            exc,
+            parsed_entity_id=parsed_entity_id,
+            payload=payload,
+        )

As per coding guidelines, "**/*.py: Keep C90 / mccabe complexity ≤ 9".

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@episodic/api/handlers.py` around lines 150 - 224, handle_update_entity is too
large/complex; extract the required-field validation loop and the exception
remapping into two small helpers so handle_update_entity becomes
orchestration-only. Create validate_required_fields(payload, required_fields)
that raises validation_error with the same message/params when a field is
missing, and create remap_update_exceptions(fn) or remap_update_errors(exc,
parsed_entity_id, payload) which contains the EntityNotFoundError and
RevisionConflictError handling currently in the try/except (use the same calls
to map_profile_template_error and preserve expected_revision extraction).
Replace the inline loop and except blocks in handle_update_entity with calls to
validate_required_fields(...) and the new remapping helper, leaving
request_builder(...), the uow/service call, and serializer_fn(...) unchanged.
episodic/api/helpers.py (1)

118-124: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Collapse pagination parsing into one field-aware helper.

Parse limit and offset once each and raise from that helper directly. The
current flow reparses both values inside _pagination_type_error_field(), which
is the duplication tripping the CodeScene gate and is easy to desynchronise
later.

♻️ Proposed refactor
+def _parse_int_query_param(
+    raw_value: str | None,
+    *,
+    name: str,
+    default: int,
+) -> int:
+    """Parse an optional integer query parameter by name."""
+    if raw_value is None:
+        return default
+    try:
+        return int(raw_value)
+    except ValueError as exc:
+        msg = f"{name} must be an integer."
+        raise validation_error(msg, field=name, constraint="type") from exc
+
+
 def parse_pagination(req: falcon.Request) -> tuple[int, int]:
     """Parse and validate common `limit`/`offset` query parameters."""
-    raw_limit = req.get_param("limit")
-    raw_offset = req.get_param("offset")
-
-    try:
-        limit = _DEFAULT_PAGE_LIMIT if raw_limit is None else int(raw_limit)
-        offset = 0 if raw_offset is None else int(raw_offset)
-    except ValueError as exc:
-        msg = "Pagination parameters limit/offset must be integers."
-        field = _pagination_type_error_field(raw_limit, raw_offset)
-        raise validation_error(msg, field=field, constraint="type") from exc
+    limit = _parse_int_query_param(
+        req.get_param("limit"),
+        name="limit",
+        default=_DEFAULT_PAGE_LIMIT,
+    )
+    offset = _parse_int_query_param(
+        req.get_param("offset"),
+        name="offset",
+        default=0,
+    )
 
     if limit < 1 or limit > _MAX_PAGE_LIMIT:
         msg = f"limit must be between 1 and {_MAX_PAGE_LIMIT}."
         raise validation_error(msg, field="limit", constraint="range")
     if offset < 0:
         msg = "offset must be a non-negative integer."
         raise validation_error(msg, field="offset", constraint="range")
     return limit, offset
-
-
-def _pagination_type_error_field(
-    raw_limit: str | None,
-    raw_offset: str | None,
-) -> str:
-    """Return the first pagination field that failed integer parsing."""
-    if raw_limit is not None:
-        try:
-            int(raw_limit)
-        except ValueError:
-            return "limit"
-    if raw_offset is not None:
-        try:
-            int(raw_offset)
-        except ValueError:
-            return "offset"
-    return "limit"

Also applies to: 159-174

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@episodic/api/helpers.py` around lines 118 - 124, Replace the current ad-hoc
parsing in parse_pagination with a small helper
_parse_int_query_param(raw_value, *, name, default) that returns default when
raw_value is None, tries int(raw_value) and on ValueError raises
validation_error(f"{name} must be an integer.", field=name, constraint="type");
then call this helper for "limit" (default _DEFAULT_PAGE_LIMIT) and "offset"
(default 0) inside parse_pagination, remove _pagination_type_error_field, and
keep the existing range checks for limit and offset unchanged (refer to
functions _parse_int_query_param, parse_pagination, and remove
_pagination_type_error_field).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@docs/developers-guide.md`:
- Line 109: Change the spelling in the REST error contract sentence that
currently reads "Every Falcon `HTTPError` raised by the canonical API is
serialised as" to use en-GB-oxendict - replace "serialised" with "serialized" so
the line becomes "Every Falcon `HTTPError` raised by the canonical API is
serialized as".

In `@episodic/api/authorization.py`:
- Around line 88-109: Replace the typing.assert_never(decision) call in the
match on AuthorizationDecision with an explicit defensive fallback that logs the
unexpected decision and returns a canonical JSON 503 response: call
_log_authorization_denial(decision, context) (or a dedicated logger) and set
resp.media to a structured error (e.g., code "internal_error", message
"Unexpected authorization decision", details {} or minimal context), set
resp.status = falcon.HTTP_503 and resp.complete = True; remove
typ.assert_never(decision) so an unexpected enum value cannot escape as an
unstructured 500.

In `@episodic/api/errors.py`:
- Around line 335-338: The except clause in the _status_code function uses
Python 2 syntax (`except IndexError, ValueError:`) which is invalid in Python 3;
change it to use a tuple of exceptions (`except (IndexError, ValueError):`) so
the block that returns 500 on parse errors executes correctly when
int(exc.status.split(...)[0]) fails.

In `@episodic/canonical/profile_templates/services/_generic.py`:
- Line 259: The inline lint suppression on the function definition
list_history_paged currently uses a bare "# noqa: PLR0913"; update it to include
a short justification (or remove it if you can refactor to reduce parameters) —
e.g. append " - justification: [brief reason]" to the noqa so it reads like the
other suppression style in this file; ensure the justification explains why the
parameter-count complexity is acceptable for list_history_paged.

In `@episodic/canonical/storage/repositories.py`:
- Around line 118-124: The paginated query orders only by
SeriesProfileRecord.created_at which can tie and cause page drift; update the
query that builds the statement (the variable named statement in the
SeriesProfileRecord select) to add a deterministic tie-breaker to the ORDER BY,
e.g. append SeriesProfileRecord.id (or the record's primary key/unique column)
after created_at so results are fully deterministic; apply the same change to
the other similar query in this file (the select around the block at the later
occurrence ordering by created_at).

In `@tests/api_fixtures.py`:
- Around line 191-195: The test currently asserts exact equality between
payload["total"] and len(items) using the _assert_equal call, which fails for
paginated responses; change the assertion to verify payload["total"] is >=
len(items) (i.e., replace the equality check with a greater-or-equal comparison)
so total represents the full filtered count while len(items) is the returned
page size; apply the same replacement for the second occurrence of the
_assert_equal check later in the file (the one at the other noted block).

In `@tests/test_api_authorization.py`:
- Around line 74-137: Collapse the three duplicate tests
(test_deny_all_authorization_returns_unauthorized_envelope,
test_forbidden_authorization_returns_forbidden_envelope,
test_authorization_adapter_exception_returns_503) into a single parametrized
pytest function that iterates over (authorization_adapter, expected_status,
expected_payload) tuples; for each case call _build_client(session_factory,
<adapter>), client.simulate_get("/v1/series-profiles"), assert
response.status_code == expected_status, and assert response.json ==
expected_payload, referencing the existing adapters DenyAllAuthorization,
ForbidSeriesProfilesAuthorization, and RaisingAuthorization to locate
implementations.

In `@tests/test_binding_resolution_api.py`:
- Around line 77-81: The new bare asserts in test_binding_resolution_api.py
should include explicit failure messages; update the assertions on
payload["limit"], payload["offset"], payload["total"] and the items-related
asserts (and the other two asserts around lines 119-120) to use assert
<condition>, "<descriptive message>" and include the actual/expected values in
the message for easier debugging (e.g., assert payload["limit"] == 20,
f"expected limit == 20, got {payload['limit']}"); locate these assertions by the
symbols payload and items in the test_binding_resolution_api.py test function
and replace each bare assert with one that supplies a clear, specific message.

In `@tests/test_reference_document_roundtrip.py`:
- Around line 60-107: Replace the bare asserts in the test (all assertions
referencing first_page_response, first_page["items"], first_page["total"],
revision_page_response, revision_page["items"], revision_page["total"],
binding_page_response, binding_page["items"], and binding_page["total"]) with
message-bearing asserts (use assert <condition>, "<descriptive message>") so
failures show deterministic context; update the checks around
support.create_reference_document_revision, simulate_get responses, and casting
validations to include clear messages like "expected status 200 for <response
variable>", "expected X items in <page variable>", or "expected total Y for
<page variable>" to comply with the assertion style guideline.

---

Outside diff comments:
In `@episodic/api/handlers.py`:
- Around line 150-224: handle_update_entity is too large/complex; extract the
required-field validation loop and the exception remapping into two small
helpers so handle_update_entity becomes orchestration-only. Create
validate_required_fields(payload, required_fields) that raises validation_error
with the same message/params when a field is missing, and create
remap_update_exceptions(fn) or remap_update_errors(exc, parsed_entity_id,
payload) which contains the EntityNotFoundError and RevisionConflictError
handling currently in the try/except (use the same calls to
map_profile_template_error and preserve expected_revision extraction). Replace
the inline loop and except blocks in handle_update_entity with calls to
validate_required_fields(...) and the new remapping helper, leaving
request_builder(...), the uow/service call, and serializer_fn(...) unchanged.

In `@episodic/api/helpers.py`:
- Around line 118-124: Replace the current ad-hoc parsing in parse_pagination
with a small helper _parse_int_query_param(raw_value, *, name, default) that
returns default when raw_value is None, tries int(raw_value) and on ValueError
raises validation_error(f"{name} must be an integer.", field=name,
constraint="type"); then call this helper for "limit" (default
_DEFAULT_PAGE_LIMIT) and "offset" (default 0) inside parse_pagination, remove
_pagination_type_error_field, and keep the existing range checks for limit and
offset unchanged (refer to functions _parse_int_query_param, parse_pagination,
and remove _pagination_type_error_field).

In `@episodic/canonical/profile_templates/services/_generic.py`:
- Around line 65-181: The function _get_repos_for_kind is too large and branches
per EntityKind (EntityKind.SERIES_PROFILE / "series_profile" and
EntityKind.EPISODE_TEMPLATE / "episode_template") with many nested local async
functions; extract each branch into its own helper factory (e.g.,
_build_series_profile_kind(uow: CanonicalUnitOfWork) -> _KindDispatch and
_build_episode_template_kind(uow: CanonicalUnitOfWork) -> _KindDispatch) that
move the repository casts, the nested async functions (_list_profiles,
_count_profiles, _list_profile_history_paged, _list_templates,
_list_template_history_paged) and the _KindDispatch construction into the
helper; then make _get_repos_for_kind a thin dispatcher that calls the
appropriate helper based on kind and returns its _KindDispatch. Ensure
signatures and returned callable types (entity_get, fetch_latest,
list_history_for_parent, list_history_for_parent_paged,
count_history_for_parent, list_entities, count_entities, get_latest_revisions)
match the originals.

In `@tests/test_api_error_envelope.py`:
- Around line 46-166: Multiple tests in tests/test_api_error_envelope.py repeat
the same arrange/act/assert pattern; consolidate them by replacing the seven
individual test_... functions with a single parametrized test using
pytest.mark.parametrize that iterates over cases (use identifiers and expected
attrs for: test_invalid_uuid_returns_validation_envelope,
test_invalid_pagination_returns_validation_envelope,
test_invalid_optional_uuid_filter_returns_validation_envelope,
test_invalid_reference_document_kind_filter_returns_validation_envelope,
test_invalid_reference_binding_target_kind_returns_validation_envelope,
test_missing_query_parameter_returns_validation_envelope,
test_unknown_identifier_returns_not_found_envelope), call
canonical_api_client.simulate_get per case, and assert via
_assert_error_envelope using the tuple fields (path, params, expected
status_code, code, optional field and constraint) so each scenario is covered
without duplicated bodies.
🪄 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 Plus

Run ID: 9df4dca3-7f70-40f8-9e7f-e31e0ef0eef8

📥 Commits

Reviewing files that changed from the base of the PR and between 348b380 and 8307d40.

📒 Files selected for processing (44)
  • docs/developers-guide.md
  • docs/episodic-podcast-generation-system-design.md
  • docs/execplans/4-1-2-finalize-rest-surfaces.md
  • docs/roadmap.md
  • docs/users-guide.md
  • episodic/api/__init__.py
  • episodic/api/app.py
  • episodic/api/authorization.py
  • episodic/api/dependencies.py
  • episodic/api/errors.py
  • episodic/api/handlers.py
  • episodic/api/helpers.py
  • episodic/api/resources/base.py
  • episodic/api/resources/episode_templates.py
  • episodic/api/resources/reference_bindings.py
  • episodic/api/resources/reference_documents.py
  • episodic/api/resources/resolved_bindings.py
  • episodic/api/resources/series_profiles.py
  • episodic/canonical/history_protocols.py
  • episodic/canonical/profile_templates/__init__.py
  • episodic/canonical/profile_templates/services/__init__.py
  • episodic/canonical/profile_templates/services/_generic.py
  • episodic/canonical/profile_templates/types.py
  • episodic/canonical/reference_documents/__init__.py
  • episodic/canonical/reference_documents/bindings.py
  • episodic/canonical/reference_documents/documents.py
  • episodic/canonical/reference_documents/revisions.py
  • episodic/canonical/reference_protocols.py
  • episodic/canonical/storage/history_repositories.py
  • episodic/canonical/storage/reference_repositories.py
  • episodic/canonical/storage/repositories.py
  • tests/api_fixtures.py
  • tests/fixtures/api.py
  • tests/test_api_authorization.py
  • tests/test_api_error_envelope.py
  • tests/test_api_route_versioning.py
  • tests/test_binding_resolution_api.py
  • tests/test_http_service_scaffold.py
  • tests/test_lifespan_hooks.py
  • tests/test_profile_template_api.py
  • tests/test_profile_template_history_pagination_api.py
  • tests/test_profile_template_pagination_api.py
  • tests/test_reference_document_api_support.py
  • tests/test_reference_document_roundtrip.py

Comment thread docs/developers-guide.md Outdated
Comment thread episodic/api/authorization.py
Comment thread episodic/api/errors.py
Comment thread episodic/canonical/profile_templates/services/_generic.py Outdated
Comment thread episodic/canonical/storage/repositories.py
Comment thread tests/api_fixtures.py Outdated
Comment thread tests/test_api_authorization.py Outdated
Comment thread tests/test_binding_resolution_api.py Outdated
Comment thread tests/test_reference_document_roundtrip.py Outdated
leynos added 12 commits May 30, 2026 02:39
Introduce a Falcon ASGI error handler that serialises canonical API
HTTP errors as the documented `{code, message, details}` envelope.
Attach validation details in shared request parsers and preserve
profile/template optimistic-lock context for stale updates.

Update route-versioning, binding-resolution, profile/template, and
reference-document tests to assert the new envelope. Document the
contract in the developer guide and record Milestone 1 progress in the
active execplan.
Add repository count methods and paged service wrappers for reusable
reference documents, document revisions, and bindings. Return `total` in
all three existing reference-domain list envelopes without changing the
legacy list service return types.

Extend reference API tests to assert totals for single-item helpers and
multi-page list responses.
Add paged profile/template repository and service reads so list
endpoints return `{items, limit, offset, total}`. Apply the same
envelope to resolved bindings and cover ordering in API tests.
Thread paged history reads through the profile/template repository,
service, and shared Falcon history resource so history endpoints return
the standard pagination envelope and validate limit/offset inputs.
Add shared optional UUID and enum query parsers, then use them
across list resources so invalid filters fail before service dispatch
with field-level validation error details.
Introduce an async authorization port, permit-all adapter, and Falcon
middleware for /v1 requests. Cover unauthorized, forbidden, bypass,
and adapter-failure envelopes while documenting the scaffold.
Document the standardized REST pagination and error envelopes,
filter parsing behaviour, and permit-all authorization scaffold across
the user guide, developer guide, and system design.
Update the canonical route smoke test to assert the final REST list
contract after the 4.1.2 pagination work.

Record the final gate findings in the ExecPlan, including the stale
assertion fix and the pre-existing guest-bios property failure that
remains outside this change.
Close out the 4.1.2 ExecPlan with final gate evidence, CodeRabbit
review results, and the known out-of-scope guest-bios property failure.

Mark roadmap item 4.1.2 complete after the final review returned zero
findings.
Apply en-GB Oxford `-ize` spellings consistently in the 4.1.2
ExecPlan after review flagged mixed suffix forms.
The previous spelling-normalization pass missed four `-ise` variants in
`docs/execplans/4-1-2-finalize-rest-surfaces.md`. Switch each to its
Oxford `-ize` form per `en-gb-oxendict`:

- `initialisation` → `initialization`
- `organisation` → `organization`
- `standardised` → `standardized`
- `serialises` → `serializes`

Greek-origin `-lyse` words (`analyse`, `paralyse`) are not used in the
file and so remain unchanged; `surprise` (and its plural form
`surprises` used in the section heading) is preserved because it is
Old-French-derived and not subject to the `-ize` rule.
The rebase onto `origin/main` brought in main's commit `7e4631d`
("Consolidate metrics/clock ports and tighten checkpoint metrics
tests (#115) (#116)"), which independently added `.claude/` near the
top of `.gitignore`. The earlier branch-local commit
`a59c343` ("Ignore `.claude/` Claude Code harness state") had added the
same entry at the bottom of the file. Drop the now-redundant trailing
entry; main's placement is kept because it groups with the other
`.cache`-style entries.
@lodyai
lodyai Bot force-pushed the 4-1-2-finalize-rest-surfaces branch from 8307d40 to 140ed87 Compare May 30, 2026 00:49
codescene-delta-analysis[bot]

This comment was marked as outdated.

`make markdownlint` failed with MD012 on `docs/users-guide.md:200`
after the rebase onto `origin/main` left a stray double blank line
between the closing reference-documents bullet and the
`### REST API reference` heading. Drop the extra blank.
@leynos

leynos commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@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.

episodic/canonical/profile_templates/services/_generic.py

Comment on lines +81 to +135

def _build_series_profile_dispatch(uow: CanonicalUnitOfWork) -> _KindDispatch:
    """Return the dispatch table for series-profile repositories."""
    profile_repo = typ.cast("_SeriesProfileRepository", uow.series_profiles)
    profile_history_repo = typ.cast(
        "_SeriesProfileHistoryRepository",
        uow.series_profile_history,
    )

    async def _list_profiles(
        _: uuid.UUID | None,
        limit: int | None,
        offset: int,
    ) -> cabc.Sequence[object]:
        return typ.cast(
            "cabc.Sequence[object]",
            await profile_repo.list(limit=limit, offset=offset),
        )

    async def _count_profiles(_: uuid.UUID | None) -> int:
        return await profile_repo.count()

    async def _list_profile_history_paged(
        profile_id: uuid.UUID,
        limit: int,
        offset: int,
    ) -> list[object]:
        return typ.cast(
            "list[object]",
            await profile_history_repo.list_for_profile_paged(
                profile_id,
                limit=limit,
                offset=offset,
            ),
        )

    return _KindDispatch(
        human_label="Series profile",
        entity_get=typ.cast(
            "cabc.Callable[[uuid.UUID], cabc.Awaitable[object | None]]",
            profile_repo.get,
        ),
        fetch_latest=typ.cast(
            "_RevisionFetcher",
            profile_history_repo.get_latest_for_profile,
        ),
        list_history_for_parent=typ.cast(
            "cabc.Callable[[uuid.UUID], cabc.Awaitable[list[object]]]",
            profile_history_repo.list_for_profile,
        ),
        list_history_for_parent_paged=_list_profile_history_paged,
        count_history_for_parent=profile_history_repo.count_for_profile,
        list_entities=_list_profiles,
        count_entities=_count_profiles,
        get_latest_revisions=profile_history_repo.get_latest_revisions_for_profiles,
    )

❌ New issue: Code Duplication
The module contains 2 functions with similar structure: _build_episode_template_dispatch,_build_series_profile_dispatch

@leynos

leynos commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@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.

Code Duplication

tests/test_api_error_envelope.py:

What lead to degradation?

The module contains 6 functions with similar structure: test_invalid_optional_uuid_filter_returns_validation_envelope,test_invalid_pagination_returns_validation_envelope,test_invalid_reference_binding_target_kind_returns_validation_envelope,test_invalid_reference_document_kind_filter_returns_validation_envelope 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

leynos commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@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.

Code Duplication

episodic/canonical/profile_templates/types.py:

What lead to degradation?

The module contains 2 functions with similar structure: init,count_for_template

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

leynos commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@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.

Large Method

tests/test_binding_resolution_api.py: test_resolved_bindings_endpoint_returns_resolved_payloads

What lead to degradation?

test_resolved_bindings_endpoint_returns_resolved_payloads has 79 lines, threshold = 70

Why does this problem occur?

Overly long functions make the code harder to read. The recommended maximum function length for the Python language is 70 lines of code. Severity: Brain Method - Complex Method - Long Method.

How to fix it?

We recommend to be careful here -- just splitting long functions don't necessarily make the code easier to read. Instead, look for natural chunks inside the functions that expresses a specific task or concern. Often, such concerns are indicated by a Code Comment followed by an if-statement. Use the EXTRACT FUNCTION refactoring to encapsulate that concern.

@leynos

leynos commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@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.

Large Method

tests/test_reference_document_roundtrip.py: test_reference_document_lists_report_total_across_pages

What lead to degradation?

test_reference_document_lists_report_total_across_pages has 86 lines, threshold = 70

Why does this problem occur?

Overly long functions make the code harder to read. The recommended maximum function length for the Python language is 70 lines of code. Severity: Brain Method - Complex Method - Long Method.

How to fix it?

We recommend to be careful here -- just splitting long functions don't necessarily make the code easier to read. Instead, look for natural chunks inside the functions that expresses a specific task or concern. Often, such concerns are indicated by a Code Comment followed by an if-statement. Use the EXTRACT FUNCTION refactoring to encapsulate that concern.

@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.

@coderabbitai

This comment was marked as resolved.

codescene-delta-analysis[bot]

This comment was marked as outdated.

`test_reference_document_lists_report_total_across_pages` exceeded the
70-line CodeScene threshold for a single function because it interleaved
setup, request, and assertion code for three distinct list endpoints
(reference documents, revisions, bindings). Extract one helper per stage:

- `_assert_document_page_total(client, profile_id) -> list[str]`
  creates three reference documents and asserts the first page of
  `/v1/series-profiles/{profile_id}/reference-documents`.
- `_assert_revision_page_total(client, profile_id, document_id) -> list[str]`
  creates two revisions on the given document and asserts the first page of
  `.../reference-documents/{document_id}/revisions`.
- `_assert_binding_page_total(client, revision_ids, template_id) -> None`
  binds each revision to the template and asserts the first page of
  `/v1/reference-bindings`.

The test function now reads top-to-bottom in 12 lines: build fixture →
document page → revision page → binding page. The sibling test
`test_reference_document_round_trip_and_binding_workflow` is untouched.
Behaviour is unchanged; both tests pass in ~5 s.
codescene-delta-analysis[bot]

This comment was marked as outdated.

`test_resolved_bindings_endpoint_returns_resolved_payloads` ran roughly
79 lines because it interleaved three concerns inside one test body:
creating a series-profile binding, creating an episode-template binding,
and asserting the pagination envelope plus item content of the
resolved-bindings response. Extract one helper per concern:

- `_create_series_profile_binding(client, profile_id, episode_id) -> str`
  creates the style-guide document, one revision, and the series-profile
  binding, returning the revision id.
- `_create_episode_template_binding(client, profile_id, template_id) -> str`
  creates the guest-profile document, one revision, and the
  episode-template binding, returning the revision id.
- `_assert_resolved_bindings_payload(payload, series_revision_id,
  template_revision_id)` asserts the limit/offset/total envelope and the
  per-item revision and document content.

The test now reads top-to-bottom in ~53 lines: build fixture, create
episode, create the two bindings via the new helpers, request the
resolved-bindings endpoint, and assert. Sibling tests in the file are
untouched. Behaviour is unchanged; all 8 tests in the file still pass in
~17 s.
codescene-delta-analysis[bot]

This comment was marked as outdated.

CodeScene flagged seven structurally identical test functions in
`tests/test_api_error_envelope.py`. Each ran the same two-step body —
`canonical_api_client.simulate_get(path, params=...)` followed by
`_assert_error_envelope(response, _Expected(...))` — and varied only in
the URL path, optional query params, and the `_Expected` envelope.

Collapse them into a single `test_error_envelope` driven by
`@pytest.mark.parametrize`. Each `pytest.param` carries the original
inputs and an explicit `id=` matching the prior test's intent
(`invalid_uuid_path_segment`, `invalid_pagination_bounds`,
`invalid_optional_uuid_filter`, `invalid_reference_document_kind_filter`,
`invalid_reference_binding_target_kind`,
`missing_required_query_parameter`, `unknown_identifier`), so failing
cases still report a descriptive identifier.

The `_Expected` dataclass and `_assert_error_envelope` helper are
untouched; only the import block grows by one line (`import pytest`).
All seven parametrised cases pass. No external module imported any of
the removed function names.
codescene-delta-analysis[bot]

This comment was marked as outdated.

CodeScene flagged the two private helpers
`_build_series_profile_dispatch` and `_build_episode_template_dispatch`
as a code-duplication finding: they shared structure but diverged on
the entity kind. Inline each builder's body directly into the
corresponding match arm of `_get_repos_for_kind`, then delete both
helpers. The closure names (`_list_entities`, `_count_entities`,
`_list_history_paged`) are intentionally identical across the two arms;
only one arm runs at runtime, so the shared names cause no collision.

Two implementation notes:

- The episode-template arm now wraps `template_repo.count` in an async
  closure that threads `series_profile_id` through to
  `template_repo.count(series_profile_id)`. The previous helper passed
  the bound method directly, which worked because
  `template_repo.count` already accepts the same parameter, but the
  explicit closure mirrors the series-profile arm and matches the
  task's target shape.
- Per-arm history-repository casts use distinct names
  (`profile_history_repo`, `template_history_repo`) rather than a
  shared `history_repo`. Python's `match` does not introduce a fresh
  scope per arm, so a shared name would be unioned by static analysis
  and the per-arm closures could not resolve their kind-specific
  methods (`list_for_profile_paged` vs `list_for_template_paged`). The
  short comment at the top of the function records this for future
  readers.

`_get_repos_for_kind` carries a single targeted suppression,
`# noqa: C901`, with a justification tying the lift in cyclomatic
complexity to this design choice (extraction was just removed at the
reviewer's request). This is the rule's last-resort case: the design
and the linter conflict, and the design has been explicitly chosen.

Behaviour is unchanged. Full test suite: 782 passed, 3 skipped.
codescene-delta-analysis[bot]

This comment was marked as outdated.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (6)
tests/test_binding_resolution_api.py (1)

19-71: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Address the code duplication flagged by CodeScene.

Both _create_series_profile_binding and _create_episode_template_binding follow the same pattern: create a document with revision, create a binding, and return the revision ID. The pipeline failure indicates this duplication violates the quality gate.

Extract a common helper that accepts the document spec and a binding-creator callback, or parametrise the shared logic.

Proposed refactor
+def _create_binding_with_document(
+    client: testing.TestClient,
+    profile_id: str,
+    doc_spec: binding_support.DocumentSpec,
+    create_binding: cabc.Callable[[str], None],
+) -> str:
+    """Create a reference document with one revision and bind it.
+
+    Returns the revision ID.
+    """
+    _, revision_id = binding_support.create_document_with_revision(
+        client,
+        profile_id,
+        doc_spec,
+    )
+    create_binding(revision_id)
+    return revision_id
+
+
 def _create_series_profile_binding(
     client: testing.TestClient,
     profile_id: str,
     episode_id: str,
 ) -> str:
     """Create a style-guide document, one revision, and a series-profile binding.
 
     Returns the revision ID.
     """
-    _, revision_id = binding_support.create_document_with_revision(
-        client,
-        profile_id,
-        binding_support.DocumentSpec(
-            kind="style_guide",
-            name="Resolved series guide",
-            summary="Resolved series guide",
-            content_hash="resolved-bindings-series",
-        ),
-    )
-    binding_support.create_series_binding(
-        client,
-        revision_id=revision_id,
-        profile_id=profile_id,
-        effective_from_episode_id=episode_id,
+    return _create_binding_with_document(
+        client,
+        profile_id,
+        binding_support.DocumentSpec(
+            kind="style_guide",
+            name="Resolved series guide",
+            summary="Resolved series guide",
+            content_hash="resolved-bindings-series",
+        ),
+        lambda rev_id: binding_support.create_series_binding(
+            client,
+            revision_id=rev_id,
+            profile_id=profile_id,
+            effective_from_episode_id=episode_id,
+        ),
     )
-    return revision_id
 
 
 def _create_episode_template_binding(
     client: testing.TestClient,
     profile_id: str,
     template_id: str,
 ) -> str:
     """Create a guest-profile document, one revision, and an episode-template binding.
 
     Returns the revision ID.
     """
-    _, revision_id = binding_support.create_document_with_revision(
-        client,
-        profile_id,
-        binding_support.DocumentSpec(
-            kind="guest_profile",
-            name="Resolved template guest",
-            summary="Resolved template guest",
-            content_hash="resolved-bindings-template",
-        ),
-    )
-    reference_support.create_reference_binding(
-        client,
-        revision_id=revision_id,
-        template_id=template_id,
+    return _create_binding_with_document(
+        client,
+        profile_id,
+        binding_support.DocumentSpec(
+            kind="guest_profile",
+            name="Resolved template guest",
+            summary="Resolved template guest",
+            content_hash="resolved-bindings-template",
+        ),
+        lambda rev_id: reference_support.create_reference_binding(
+            client,
+            revision_id=rev_id,
+            template_id=template_id,
+        ),
     )
-    return revision_id

Add the import for cabc:

 if typ.TYPE_CHECKING:
     import asyncio
+    import collections.abc as cabc
 
     from falcon import testing
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_binding_resolution_api.py` around lines 19-71, extract the
duplicated document-and-binding creation pattern from _create_series_profile_binding
and _create_episode_template_binding into a new helper _create_binding_with_document
that accepts (client, profile_id, doc_spec, create_binding_callback). Update both
functions to call this new helper, passing their specific DocumentSpec and a lambda
that wraps their binding creation call. Import collections.abc as cabc in the
TYPE_CHECKING block to type the callback parameter.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/test_binding_resolution_api.py` around lines 19 - 71, Duplicate logic
in _create_series_profile_binding and _create_episode_template_binding should be
extracted into a helper: add _create_binding_with_document(client, profile_id,
doc_spec, create_binding_callback) that calls
binding_support.create_document_with_revision to get revision_id, then invokes
create_binding_callback(client, revision_id) and returns revision_id; update
_create_series_profile_binding to call the helper with its specific DocumentSpec
and a lambda that calls binding_support.create_series_binding(...,
revision_id=revision_id, profile_id=profile_id,
effective_from_episode_id=episode_id), and update
_create_episode_template_binding to call the helper with its DocumentSpec and a
lambda that calls reference_support.create_reference_binding(...,
revision_id=revision_id, template_id=template_id); add import collections.abc as
cabc inside the TYPE_CHECKING block and annotate the callback parameter using
cabc.Callable for typing.
episodic/canonical/reference_documents/documents.py (1)

92-109: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Expand the docstring for list_reference_documents_paged to full NumPy style.

This public function requires structured documentation. Add Parameters, Returns, and Raises sections consistent with list_reference_documents (lines 77–89).

📝 Suggested docstring structure
 async def list_reference_documents_paged(
     uow: CanonicalUnitOfWork,
     *,
     request: ReferenceDocumentListRequest,
 ) -> tuple[list[ReferenceDocument], int]:
-    """List reusable reference documents and their unpaginated total."""
+    """List reusable reference documents for one owning series profile and their unpaginated total.
+
+    Parameters
+    ----------
+    uow : CanonicalUnitOfWork
+        Unit of work providing repository access.
+    request : ReferenceDocumentListRequest
+        Typed list request containing owner series identifier, optional kind filter,
+        and pagination values.
+
+    Returns
+    -------
+    tuple[list[ReferenceDocument], int]
+        Documents matching the requested owner series and pagination window, plus the
+        unpaginated total count of matching documents.
+
+    Raises
+    ------
+    ReferenceValidationError
+        If pagination values, owner series identifier, or kind are invalid.
+    ReferenceEntityNotFoundError
+        If the owning series does not exist.
+    """
     parsed_owner_id, parsed_kind = await _prepare_document_list_query(uow, request)

As per coding guidelines: "Docstrings must follow the numpy style guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/reference_documents/documents.py` at line 92, expand the single-line docstring for `list_reference_documents_paged` into a full NumPy-style docstring with Parameters, Returns, and Raises sections; clarify that this variant returns a tuple of (documents list, total count); document `uow` and `request` in Parameters, the tuple structure in Returns, and list ReferenceValidationError and ReferenceEntityNotFoundError in Raises.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/reference_documents/documents.py` around lines 92 - 109,
Update the docstring for the public function list_reference_documents_paged to a
full NumPy-style docstring: replace the single-line summary with a short
description and add Parameters (documenting uow: CanonicalUnitOfWork and
request: ReferenceDocumentListRequest), a Returns section describing
tuple[list[ReferenceDocument], int] (documents for the requested owner series
and the unpaginated total), and a Raises section listing
ReferenceValidationError and ReferenceEntityNotFoundError; keep wording
consistent with the existing list_reference_documents docstring and ensure the
docstring sits immediately above the list_reference_documents_paged definition.
episodic/canonical/profile_templates/services/_generic.py (2)

273-286: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add a full NumPy-style docstring to list_history_paged.

This public function is missing documentation entirely. Add Parameters, Returns, and Raises sections.

📝 Suggested docstring structure
 async def list_history_paged(
     uow: CanonicalUnitOfWork,
     *,
     parent_id: uuid.UUID,
     kind: EntityKind | str,
     page: Pagination,
 ) -> tuple[list[object], int]:
+    """List paged history entries and the unpaginated total for one parent entity.
+
+    Parameters
+    ----------
+    uow : CanonicalUnitOfWork
+        Unit-of-work providing repositories and transactional boundaries.
+    parent_id : uuid.UUID
+        Identifier of the parent entity.
+    kind : EntityKind | str
+        Entity kind selector (series profile or episode template).
+    page : Pagination
+        Pagination parameters (limit and offset).
+
+    Returns
+    -------
+    tuple[list[object], int]
+        History entries for the requested parent entity within the pagination window,
+        plus the unpaginated total count of history entries for that parent.
+
+    Raises
+    ------
+    ValueError
+        Raised when ``kind`` is unsupported.
+    """
     dispatch = _get_repos_for_kind(uow, kind)

As per coding guidelines: "Docstrings must follow the numpy style guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/profile_templates/services/_generic.py` at line 273, add a full NumPy-style docstring to `list_history_paged` with Parameters, Returns, and Raises sections; document `uow`, `parent_id`, `kind`, and `page` in Parameters; clarify in Returns that the tuple contains (history items list, total count); list ValueError in Raises for unsupported kind.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/profile_templates/services/_generic.py` around lines 273 -
286, Add a NumPy-style docstring to the public function list_history_paged
describing its purpose and documenting Parameters (uow: CanonicalUnitOfWork,
parent_id: uuid.UUID, kind: EntityKind | str, page: Pagination), Returns
(tuple[list[object], int] — the paged history entries and the unpaginated total
count), and Raises (ValueError for unsupported kind); place the docstring
immediately above the function signature and follow the numpy sections
"Parameters", "Returns", and "Raises" wording and formatting as in the project's
docstring conventions.

325-340: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Add a full NumPy-style docstring to list_entities_with_revisions_paged.

This public function is missing documentation entirely. Add Parameters, Returns, and Raises sections mirroring the pattern from list_entities_with_revisions (lines 289–322).

📝 Suggested docstring structure
 async def list_entities_with_revisions_paged(
     uow: CanonicalUnitOfWork,
     *,
     kind: EntityKind | str,
     page: Pagination,
     series_profile_id: uuid.UUID | None = None,
 ) -> tuple[list[tuple[object, int]], int]:
+    """List entities paired with their latest revisions and the unpaginated total.
+
+    Parameters
+    ----------
+    uow : CanonicalUnitOfWork
+        Unit-of-work providing repositories and transactional boundaries.
+    kind : EntityKind | str
+        Entity kind selector (series profile or episode template).
+    page : Pagination
+        Pagination parameters (limit and offset).
+    series_profile_id : uuid.UUID | None, default None
+        Optional profile filter used for episode-template listing.
+
+    Returns
+    -------
+    tuple[list[tuple[object, int]], int]
+        Sequence of ``(entity, latest_revision)`` pairs within the pagination window,
+        plus the unpaginated total count of matching entities.
+
+    Raises
+    ------
+    ValueError
+        Raised when ``kind`` is unsupported.
+    """
     dispatch = _get_repos_for_kind(uow, kind)

As per coding guidelines: "Docstrings must follow the numpy style guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/profile_templates/services/_generic.py` at line 325, add a full NumPy-style docstring to `list_entities_with_revisions_paged` with Parameters, Returns, and Raises sections; document `uow`, `kind`, `page`, and `series_profile_id` in Parameters; clarify in Returns that the tuple contains (list of (entity, revision) pairs, total count); list ValueError in Raises for unsupported kind.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/profile_templates/services/_generic.py` around lines 325 -
340, Add a full NumPy-style docstring to the public function
list_entities_with_revisions_paged describing purpose, Parameters (uow:
CanonicalUnitOfWork, kind: EntityKind | str, page: Pagination,
series_profile_id: uuid.UUID | None = None), Returns (tuple[list[tuple[object,
int]], int] where the list is (entity, latest_revision) pairs and the int is the
unpaginated total), and Raises (ValueError for unsupported kind); follow the
exact structure and wording style used in list_entities_with_revisions to ensure
consistency.
episodic/canonical/reference_documents/bindings.py (1)

429-448: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Expand the docstring for list_reference_bindings_paged to full NumPy style.

This public function requires structured documentation matching list_reference_bindings (lines 394–417). Add Parameters, Returns, and Raises sections.

📝 Suggested docstring structure
 async def list_reference_bindings_paged(
     uow: CanonicalUnitOfWork,
     *,
     request: ReferenceBindingListRequest,
 ) -> tuple[list[ReferenceBinding], int]:
-    """List reusable reference bindings and their unpaginated total."""
+    """List reusable reference bindings for one target context and their unpaginated total.
+
+    Parameters
+    ----------
+    uow : CanonicalUnitOfWork
+        Unit of work providing repository access.
+    request : ReferenceBindingListRequest
+        Typed list request containing target identifiers and pagination values.
+
+    Returns
+    -------
+    tuple[list[ReferenceBinding], int]
+        Bindings matching the requested target context and pagination window,
+        plus the unpaginated total count of matching bindings.
+
+    Raises
+    ------
+    ReferenceValidationError
+        If pagination values, target kind, or target identifier are invalid.
+    """
     _validate_pagination(request.limit, request.offset)

As per coding guidelines: "Docstrings must follow the numpy style guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/reference_documents/bindings.py` at line 429, expand the single-line docstring for `list_reference_bindings_paged` into a full NumPy-style docstring with Parameters, Returns, and Raises sections; model it after the existing `list_reference_bindings` docstring (lines 394-417) but clarify that this variant returns a tuple of (bindings list, total count); ensure the Parameters section documents `uow` and `request`, the Returns section documents the tuple structure, and the Raises section lists ReferenceValidationError.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/reference_documents/bindings.py` around lines 429 - 448,
The docstring for list_reference_bindings_paged is currently a single-line
summary; update it to a full NumPy-style docstring (modeled on
list_reference_bindings) that includes Parameters (documenting uow:
CanonicalUnitOfWork and request: ReferenceBindingListRequest), Returns
(tuple[list[ReferenceBinding], int] describing the bindings list and the
unpaginated total), and Raises (ReferenceValidationError for invalid
pagination/target inputs); keep wording concise and consistent with the existing
list_reference_bindings docstring.
episodic/canonical/reference_documents/revisions.py (1)

84-99: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Expand the docstring for list_reference_document_revisions_paged to full NumPy style.

This public function requires structured documentation. Add Parameters, Returns, and Raises sections mirroring the pattern from list_reference_document_revisions (lines 70–81).

📝 Suggested docstring structure
 async def list_reference_document_revisions_paged(
     uow: CanonicalUnitOfWork,
     *,
     request: ReferenceDocumentRevisionListRequest,
 ) -> tuple[list[ReferenceDocumentRevision], int]:
-    """List immutable revisions and their unpaginated total."""
+    """List immutable revisions for one reference document and their unpaginated total.
+
+    Parameters
+    ----------
+    uow : CanonicalUnitOfWork
+        Unit of work providing repository access.
+    request : ReferenceDocumentRevisionListRequest
+        Typed list request containing document identifiers and pagination values.
+
+    Returns
+    -------
+    tuple[list[ReferenceDocumentRevision], int]
+        Revisions matching the requested document and pagination window, plus the
+        unpaginated total count of revisions for that document.
+
+    Raises
+    ------
+    ReferenceValidationError
+        If pagination values or identifiers are invalid.
+    ReferenceEntityNotFoundError
+        If the referenced document does not exist or does not match the owner scope.
+    """
     parsed_document_id = await _prepare_revision_list_query(uow, request)

As per coding guidelines: "Docstrings must follow the numpy style guide. Use a single-line summary for private functions and methods, and full structured docs for all public interfaces."

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/reference_documents/revisions.py` at line 84, expand the single-line docstring for `list_reference_document_revisions_paged` into a full NumPy-style docstring with Parameters, Returns, and Raises sections; clarify that this variant returns a tuple of (revisions list, total count); document `uow` and `request` in Parameters, the tuple structure in Returns, and list ReferenceValidationError and ReferenceEntityNotFoundError in Raises.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@episodic/canonical/reference_documents/revisions.py` around lines 84 - 99,
Update the single-line docstring for list_reference_document_revisions_paged to
a full NumPy-style docstring matching the pattern used in
list_reference_document_revisions: add a one-line summary then Parameters
documenting uow: CanonicalUnitOfWork and request:
ReferenceDocumentRevisionListRequest, a Returns section describing
tuple[list[ReferenceDocumentRevision], int] (revisions for the requested
document within the pagination window and the unpaginated total), and a Raises
section listing ReferenceValidationError and ReferenceEntityNotFoundError; keep
wording consistent with the other function and place the expanded docstring
immediately above the function definition in revisions.py.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/fixtures/llm.py`:
- Line 122: The function _build_adapter currently accepts five parameters but
the noqa suppression removed PLR0913; restore PLR0913 in the noqa list for the
_build_adapter definition (i.e., change "noqa: TD001, TD002" to include PLR0913)
so the linters are consistent with the existing pylint disable, or alternatively
refactor _build_adapter to accept four or fewer arguments and remove both
PLR0913 and the pylint disable if you choose to reduce parameters; target the
async def _build_adapter(...) signature when making the change.

---

Outside diff comments:
In `@episodic/canonical/profile_templates/services/_generic.py`:
- Around line 273-286: Add a NumPy-style docstring to the public function
list_history_paged describing its purpose and documenting Parameters (uow:
CanonicalUnitOfWork, parent_id: uuid.UUID, kind: EntityKind | str, page:
Pagination), Returns (tuple[list[object], int] — the paged history entries and
the unpaginated total count), and Raises (ValueError for unsupported kind);
place the docstring immediately above the function signature and follow the
numpy sections "Parameters", "Returns", and "Raises" wording and formatting as
in the project's docstring conventions.
- Around line 325-340: Add a full NumPy-style docstring to the public function
list_entities_with_revisions_paged describing purpose, Parameters (uow:
CanonicalUnitOfWork, kind: EntityKind | str, page: Pagination,
series_profile_id: uuid.UUID | None = None), Returns (tuple[list[tuple[object,
int]], int] where the list is (entity, latest_revision) pairs and the int is the
unpaginated total), and Raises (ValueError for unsupported kind); follow the
exact structure and wording style used in list_entities_with_revisions to ensure
consistency.

In `@episodic/canonical/reference_documents/bindings.py`:
- Around line 429-448: The docstring for list_reference_bindings_paged is
currently a single-line summary; update it to a full NumPy-style docstring
(modeled on list_reference_bindings) that includes Parameters (documenting uow:
CanonicalUnitOfWork and request: ReferenceBindingListRequest), Returns
(tuple[list[ReferenceBinding], int] describing the bindings list and the
unpaginated total), and Raises (ReferenceValidationError for invalid
pagination/target inputs); keep wording concise and consistent with the existing
list_reference_bindings docstring.

In `@episodic/canonical/reference_documents/documents.py`:
- Around line 92-109: Update the docstring for the public function
list_reference_documents_paged to a full NumPy-style docstring: replace the
single-line summary with a short description and add Parameters (documenting
uow: CanonicalUnitOfWork and request: ReferenceDocumentListRequest), a Returns
section describing tuple[list[ReferenceDocument], int] (documents for the
requested owner series and the unpaginated total), and a Raises section listing
ReferenceValidationError and ReferenceEntityNotFoundError; keep wording
consistent with the existing list_reference_documents docstring and ensure the
docstring sits immediately above the list_reference_documents_paged definition.

In `@episodic/canonical/reference_documents/revisions.py`:
- Around line 84-99: Update the single-line docstring for
list_reference_document_revisions_paged to a full NumPy-style docstring matching
the pattern used in list_reference_document_revisions: add a one-line summary
then Parameters documenting uow: CanonicalUnitOfWork and request:
ReferenceDocumentRevisionListRequest, a Returns section describing
tuple[list[ReferenceDocumentRevision], int] (revisions for the requested
document within the pagination window and the unpaginated total), and a Raises
section listing ReferenceValidationError and ReferenceEntityNotFoundError; keep
wording consistent with the other function and place the expanded docstring
immediately above the function definition in revisions.py.

In `@tests/test_binding_resolution_api.py`:
- Around line 19-71: Duplicate logic in _create_series_profile_binding and
_create_episode_template_binding should be extracted into a helper: add
_create_binding_with_document(client, profile_id, doc_spec,
create_binding_callback) that calls
binding_support.create_document_with_revision to get revision_id, then invokes
create_binding_callback(client, revision_id) and returns revision_id; update
_create_series_profile_binding to call the helper with its specific DocumentSpec
and a lambda that calls binding_support.create_series_binding(...,
revision_id=revision_id, profile_id=profile_id,
effective_from_episode_id=episode_id), and update
_create_episode_template_binding to call the helper with its DocumentSpec and a
lambda that calls reference_support.create_reference_binding(...,
revision_id=revision_id, template_id=template_id); add import collections.abc as
cabc inside the TYPE_CHECKING block and annotate the callback parameter using
cabc.Callable for typing.
🪄 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 Plus

Run ID: f88f8737-6ea3-426f-a3d5-c485ac30a76a

📥 Commits

Reviewing files that changed from the base of the PR and between 552e97b and 655c0ac.

📒 Files selected for processing (12)
  • episodic/api/handlers.py
  • episodic/canonical/profile_templates/services/_generic.py
  • episodic/canonical/reference_documents/bindings.py
  • episodic/canonical/reference_documents/documents.py
  • episodic/canonical/reference_documents/revisions.py
  • pyproject.toml
  • tests/fixtures/api.py
  • tests/fixtures/llm.py
  • tests/test_api_error_envelope.py
  • tests/test_binding_resolution_api.py
  • tests/test_profile_template_pagination_api.py
  • tests/test_reference_document_roundtrip.py

Comment thread tests/fixtures/llm.py
@leynos

leynos commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@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.

Code Duplication

tests/test_binding_resolution_api.py:

What lead to degradation?

The module contains 2 functions with similar structure: _create_episode_template_binding,_create_series_profile_binding

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

leynos commented May 31, 2026

Copy link
Copy Markdown
Owner Author

@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.

Large Method

episodic/canonical/profile_templates/services/_generic.py: _get_repos_for_kind

What lead to degradation?

_get_repos_for_kind has 115 lines, threshold = 70

Why does this problem occur?

Overly long functions make the code harder to read. The recommended maximum function length for the Python language is 70 lines of code. Severity: Brain Method - Complex Method - Long Method.

How to fix it?

We recommend to be careful here -- just splitting long functions don't necessarily make the code easier to read. Instead, look for natural chunks inside the functions that expresses a specific task or concern. Often, such concerns are indicated by a Code Comment followed by an if-statement. Use the EXTRACT FUNCTION refactoring to encapsulate that concern.

@coderabbitai

coderabbitai Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Tip

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

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
cat tests/test_binding_resolution_api.py

Length of output: 9213



File: tests/test_binding_resolution_api.py
Symbols: _create_series_profile_binding, _create_episode_template_binding


Do not refactor this further. The two helpers were deliberately introduced in the prior extract-function refactor to shorten test_resolved_bindings_endpoint_returns_resolved_payloads. Their structural similarity is load-bearing: the test exercises resolution of both binding targets simultaneously, so the setup for each target is inherently parallel in shape.

The binding calls differ in support module, keyword argument set, and domain meaning — binding_support.create_series_binding takes profile_id and effective_from_episode_id, while reference_support.create_reference_binding takes template_id. A shared generic helper would require injecting the binding callable and its varying keyword arguments as parameters, producing code that is harder to read and harder to modify independently if either binding kind's setup changes.

Paste the following into the CodeScene suppression dialogue:


These two helpers (_create_series_profile_binding and _create_episode_template_binding) are intentionally parallel. They exist to set up the two distinct binding targets exercised by a single integration test — a series-profile episode anchor and an episode-template anchor — which are parallel domain concepts by design. Their shared structure (create document → create revision → create binding → return revision ID) mirrors the parallel structure of the domain, not a DRY violation.

The binding calls they encapsulate come from different support modules and carry different keyword-argument signatures (profile_id + effective_from_episode_id versus template_id). Merging them into a shared abstraction would require threading the binding callable and its varying kwargs as parameters, producing a harder-to-read facade that conflates two distinct domain operations. Each helper must remain independently modifiable if the setup for either binding kind evolves.

@coderabbitai

coderabbitai Bot commented May 31, 2026

Copy link
Copy Markdown
Contributor

Tip

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

File: episodic/canonical/profile_templates/services/_generic.py
Symbol: _get_repos_for_kind


Do not refactor this. The Large Method finding is a direct and unavoidable consequence of resolving the Code Duplication finding on _build_series_profile_dispatch and _build_episode_template_dispatch. The inlining of both builders into _get_repos_for_kind as match arms was the correct fix for that earlier diagnostic. Re-extracting them into named functions would immediately re-trigger the duplication finding — the two remediations are mutually exclusive.

Paste the following into the CodeScene suppression dialogue:


_get_repos_for_kind is long because it inlines per-kind repository wiring directly into two match arms rather than delegating to separate builder functions. The inlining was a deliberate resolution of a prior Code Duplication diagnostic: the two extracted builders (_build_series_profile_dispatch, _build_episode_template_dispatch) shared near-identical structure and were flagged as a DRY violation. Extracting them again would immediately re-trigger that finding, making the two diagnostics mutually exclusive.

The function contains no branching within either match arm; the line count comes from explicit typ.cast() annotations that are required for static type-safety at the call sites, not from logic complexity. Cognitive complexity is low — the function is two flat, parallel wiring blocks. The appropriate remedy is to suppress this finding rather than introduce an artificial split that would be harmful to code health overall.

codescene-delta-analysis[bot]

This comment was marked as outdated.

@leynos
leynos merged commit faf351c into main May 31, 2026
4 checks passed
@leynos
leynos deleted the 4-1-2-finalize-rest-surfaces branch May 31, 2026 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant