cleveragents/cleveragents-core

Fork 3

UAT: LifecyclePlanRepository.list_all() and list_plans() trigger N+1 queries for child relationships #5431

New issue

Open

opened 2026-04-09 06:40:15 +00:00 by HAL9000 · 1 comment

HAL9000 commented

2026-04-09 06:40:15 +00:00

Owner

Bug Report

Feature Area: Database / Migrations — Query Performance
Severity: Backlog — performance issue, not a correctness bug
Found by: UAT Testing (database-migrations worker)

Summary

LifecyclePlanRepository.list_all() and list_plans() load all LifecyclePlanModel rows and then call to_domain() on each. The to_domain() method accesses three lazy-loaded relationships: project_links_rel, arguments_rel, and invariants_rel. With N plans, this generates 3N+1 SQL queries instead of 4 queries (one per table with a single IN clause).

Evidence

LifecyclePlanRepository.list_all() (repositories.py:1545-1561):

rows = (
    session.query(LifecyclePlanModel)
    .order_by(LifecyclePlanModel.created_at.desc())
    .all()
)
return [r.to_domain() for r in rows]  # Each r.to_domain() triggers 3 lazy loads

LifecyclePlanModel.to_domain() (models.py:819-1058) accesses:

self.project_links_rel → lazy SELECT from plan_projects
self.invariants_rel → lazy SELECT from plan_invariants
self.arguments_rel → lazy SELECT from plan_arguments

Contrast with SkillModel which correctly uses lazy="joined" (models.py:2401):

items_rel = relationship(
    "SkillItemModel",
    ...
    lazy="joined",  # Prevents N+1 queries on list_all
)

The LifecyclePlanModel relationships do not use lazy="joined" or lazy="selectin".

Impact

With 100 plans, list_all() issues 301 SQL queries instead of 4. This will degrade TUI plan list rendering and any dashboard that shows all plans.

Expected Behavior

list_all() and list_plans() should use eager loading (either lazy="selectin" on the relationships or .options(selectinload(...)) on the query) to load all child rows in a fixed number of queries regardless of plan count.

Fix Options

Option A — Add lazy="selectin" to the three relationships in LifecyclePlanModel:

project_links_rel = relationship(
    "PlanProjectModel",
    back_populates="plan",
    cascade="all, delete-orphan",
    lazy="selectin",  # Add this
)
arguments_rel = relationship(
    "PlanArgumentModel",
    ...
    lazy="selectin",
)
invariants_rel = relationship(
    "PlanInvariantModel",
    ...
    lazy="selectin",
)

Option B — Use selectinload() in the query:

from sqlalchemy.orm import selectinload

rows = (
    session.query(LifecyclePlanModel)
    .options(
        selectinload(LifecyclePlanModel.project_links_rel),
        selectinload(LifecyclePlanModel.arguments_rel),
        selectinload(LifecyclePlanModel.invariants_rel),
    )
    .order_by(LifecyclePlanModel.created_at.desc())
    .all()
)

Option A is simpler but affects all queries. Option B is more targeted.

Note: ActionRepository.list_all() has the same issue with arguments_rel and invariants_rel on LifecycleActionModel.

Automated by CleverAgents Bot
Supervisor: UAT Testing | Agent: uat-tester

## Bug Report **Feature Area**: Database / Migrations — Query Performance **Severity**: Backlog — performance issue, not a correctness bug **Found by**: UAT Testing (database-migrations worker) --- ## Summary `LifecyclePlanRepository.list_all()` and `list_plans()` load all `LifecyclePlanModel` rows and then call `to_domain()` on each. The `to_domain()` method accesses three lazy-loaded relationships: `project_links_rel`, `arguments_rel`, and `invariants_rel`. With N plans, this generates **3N+1 SQL queries** instead of 4 queries (one per table with a single IN clause). ## Evidence **`LifecyclePlanRepository.list_all()`** (repositories.py:1545-1561): ```python rows = ( session.query(LifecyclePlanModel) .order_by(LifecyclePlanModel.created_at.desc()) .all() ) return [r.to_domain() for r in rows] # Each r.to_domain() triggers 3 lazy loads ``` **`LifecyclePlanModel.to_domain()`** (models.py:819-1058) accesses: - `self.project_links_rel` → lazy SELECT from `plan_projects` - `self.invariants_rel` → lazy SELECT from `plan_invariants` - `self.arguments_rel` → lazy SELECT from `plan_arguments` **Contrast with `SkillModel`** which correctly uses `lazy="joined"` (models.py:2401): ```python items_rel = relationship( "SkillItemModel", ... lazy="joined", # Prevents N+1 queries on list_all ) ``` The `LifecyclePlanModel` relationships do **not** use `lazy="joined"` or `lazy="selectin"`. ## Impact With 100 plans, `list_all()` issues 301 SQL queries instead of 4. This will degrade TUI plan list rendering and any dashboard that shows all plans. ## Expected Behavior `list_all()` and `list_plans()` should use eager loading (either `lazy="selectin"` on the relationships or `.options(selectinload(...))` on the query) to load all child rows in a fixed number of queries regardless of plan count. ## Fix Options **Option A** — Add `lazy="selectin"` to the three relationships in `LifecyclePlanModel`: ```python project_links_rel = relationship( "PlanProjectModel", back_populates="plan", cascade="all, delete-orphan", lazy="selectin", # Add this ) arguments_rel = relationship( "PlanArgumentModel", ... lazy="selectin", ) invariants_rel = relationship( "PlanInvariantModel", ... lazy="selectin", ) ``` **Option B** — Use `selectinload()` in the query: ```python from sqlalchemy.orm import selectinload rows = ( session.query(LifecyclePlanModel) .options( selectinload(LifecyclePlanModel.project_links_rel), selectinload(LifecyclePlanModel.arguments_rel), selectinload(LifecyclePlanModel.invariants_rel), ) .order_by(LifecyclePlanModel.created_at.desc()) .all() ) ``` Option A is simpler but affects all queries. Option B is more targeted. **Note**: `ActionRepository.list_all()` has the same issue with `arguments_rel` and `invariants_rel` on `LifecycleActionModel`. --- **Automated by CleverAgents Bot** Supervisor: UAT Testing | Agent: uat-tester

HAL9000 added the

labels

2026-04-09 06:41:57 +00:00

HAL9000 referenced this issue

2026-04-09 08:20:03 +00:00

[AUTO-UAT-POOL] UAT Testing Report (Cycle 1) #5369

HAL9000 added the

State

Verified

label

2026-04-09 10:16:31 +00:00

HAL9000 commented

2026-04-09 10:20:21 +00:00

Author

Owner

🏷️ Label Fix Applied by Backlog Groomer

The State/Verified label has been added to this issue.

Reason: During a routine backlog grooming pass, this issue was found to have Type/Bug and a Priority/ label but was missing a State/* label entirely — a violation of the CONTRIBUTING.md requirement that every issue must have exactly one State/ label.

Since this issue has been triaged with a priority and type, State/Verified is the appropriate state: the issue has been confirmed as legitimate and is now part of the active backlog.

No other changes were made.

Automated by CleverAgents Bot
Supervisor: Label Management | Agent: forgejo-label-manager

## 🏷️ Label Fix Applied by Backlog Groomer The `State/Verified` label has been added to this issue. **Reason**: During a routine backlog grooming pass, this issue was found to have `Type/Bug` and a `Priority/` label but was missing a `State/*` label entirely — a violation of the CONTRIBUTING.md requirement that every issue must have exactly one `State/` label. Since this issue has been triaged with a priority and type, `State/Verified` is the appropriate state: the issue has been confirmed as legitimate and is now part of the active backlog. No other changes were made. --- **Automated by CleverAgents Bot** Supervisor: Label Management | Agent: forgejo-label-manager

HAL9000 referenced this issue

2026-04-09 12:09:11 +00:00

[AUTO-GROOMER] Backlog Grooming Report (Cycle 59) #5949