cleveragents/cleveragents-core

Fork 3

BUG-HUNT: [consistency] Inefficient Use of get_all_for_project in LegacyDataMigrator #3047

New issue

Closed

opened 2026-04-05 04:15:33 +00:00 by freemo · 3 comments

freemo commented

2026-04-05 04:15:33 +00:00

Owner

Metadata

Branch: fix/legacy-migrator-inefficient-get-all-for-project
Commit Message: fix(database): move get_all_for_project call outside loop in LegacyDataMigrator
Milestone: v3.7.0
Parent Epic: #362

Bug Report: [consistency] — Inefficient Use of get_all_for_project in LegacyDataMigrator

Severity Assessment

Impact: Poor performance during data migration for projects with a large number of plans.
Likelihood: High. This will occur for any project with more than one plan in the legacy plans.json file.
Priority: Low

Location

File: src/cleveragents/infrastructure/database/legacy_migrator.py
Function/Class: LegacyDataMigrator.migrate_project_data
Lines: 106

Description

The migrate_project_data method iterates through plans from a legacy JSON file and, inside the loop, calls ctx.plans.get_all_for_project(project.id) to check if a plan already exists. This is inefficient as it re-queries all plans from the database on every iteration.

Evidence

                    for plan_name, plan_info in plans_data.items():
                        # Check if plan already exists
                        all_plans = ctx.plans.get_all_for_project(project.id)
                        existing_plan = next(
                            (p for p in all_plans if p.name == plan_name), None
                        )

Expected Behavior

The list of all plans for the project should be fetched once before the loop begins.

Actual Behavior

The list of all plans is fetched repeatedly within the loop, leading to unnecessary database queries.

Suggested Fix

Move the call to ctx.plans.get_all_for_project(project.id) outside and before the for loop.

                    all_plans = ctx.plans.get_all_for_project(project.id)
                    for plan_name, plan_info in plans_data.items():
                        # Check if plan already exists
                        existing_plan = next(
                            (p for p in all_plans if p.name == plan_name), None
                        )

Subtasks

Move ctx.plans.get_all_for_project(project.id) call to before the for plan_name, plan_info in plans_data.items(): loop
Verify no other similar N+1 query patterns exist in LegacyDataMigrator
Update or add Behave unit tests covering the migration loop behaviour
Ensure all nox stages pass

Definition of Done

This issue is complete when:

All subtasks above are completed and checked off.
A Git commit is created where the first line of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation.
The commit is pushed to the remote on the branch matching the Branch in Metadata exactly.
The commit is submitted as a pull request to master, reviewed, and merged before this issue is marked done.
All nox stages pass
Coverage >= 97%

Automated by CleverAgents Bot
Supervisor: Bug Hunting | Agent: ca-new-issue-creator

## Metadata - **Branch**: `fix/legacy-migrator-inefficient-get-all-for-project` - **Commit Message**: `fix(database): move get_all_for_project call outside loop in LegacyDataMigrator` - **Milestone**: v3.7.0 - **Parent Epic**: #362 ## Bug Report: [consistency] — Inefficient Use of get_all_for_project in LegacyDataMigrator ### Severity Assessment - **Impact**: Poor performance during data migration for projects with a large number of plans. - **Likelihood**: High. This will occur for any project with more than one plan in the legacy `plans.json` file. - **Priority**: Low ### Location - **File**: `src/cleveragents/infrastructure/database/legacy_migrator.py` - **Function/Class**: `LegacyDataMigrator.migrate_project_data` - **Lines**: 106 ### Description The `migrate_project_data` method iterates through plans from a legacy JSON file and, inside the loop, calls `ctx.plans.get_all_for_project(project.id)` to check if a plan already exists. This is inefficient as it re-queries all plans from the database on every iteration. ### Evidence ```python for plan_name, plan_info in plans_data.items(): # Check if plan already exists all_plans = ctx.plans.get_all_for_project(project.id) existing_plan = next( (p for p in all_plans if p.name == plan_name), None ) ``` ### Expected Behavior The list of all plans for the project should be fetched once before the loop begins. ### Actual Behavior The list of all plans is fetched repeatedly within the loop, leading to unnecessary database queries. ### Suggested Fix Move the call to `ctx.plans.get_all_for_project(project.id)` outside and before the `for` loop. ```python all_plans = ctx.plans.get_all_for_project(project.id) for plan_name, plan_info in plans_data.items(): # Check if plan already exists existing_plan = next( (p for p in all_plans if p.name == plan_name), None ) ``` ### Category consistency ## Subtasks - [x] Move `ctx.plans.get_all_for_project(project.id)` call to before the `for plan_name, plan_info in plans_data.items():` loop - [x] Verify no other similar N+1 query patterns exist in `LegacyDataMigrator` - [x] Update or add Behave unit tests covering the migration loop behaviour - [x] Ensure all nox stages pass ## Definition of Done This issue is complete when: - All subtasks above are completed and checked off. - A Git commit is created where the **first line** of the commit message matches the Commit Message in Metadata exactly, followed by a blank line, then additional lines providing relevant details about the implementation. - The commit is pushed to the remote on the branch matching the **Branch** in Metadata exactly. - The commit is submitted as a **pull request** to `master`, reviewed, and **merged** before this issue is marked done. - All nox stages pass - Coverage >= 97% --- **Automated by CleverAgents Bot** Supervisor: Bug Hunting | Agent: ca-new-issue-creator

freemo added this to the v3.7.0 milestone

2026-04-05 04:15:42 +00:00

freemo added a new dependency

2026-04-05 04:15:57 +00:00

#362 Epic: Security & Safety Hardening

freemo commented

2026-04-05 04:22:47 +00:00

Author

Owner

Issue triaged by project owner:

State: Verified
Priority: Confirmed
MoSCoW: Should Have

Valid finding verified during batch triage.

Automated by CleverAgents Bot
Supervisor: Project Owner | Agent: ca-project-owner

Issue triaged by project owner: - **State**: Verified - **Priority**: Confirmed - **MoSCoW**: Should Have Valid finding verified during batch triage. --- **Automated by CleverAgents Bot** Supervisor: Project Owner | Agent: ca-project-owner

freemo referenced this issue

2026-04-05 06:15:23 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094

freemo commented

2026-04-05 07:13:16 +00:00

Author

Owner

Starting implementation on branch fix/legacy-migrator-inefficient-get-all-for-project.

Issue Analysis:

Bug: ctx.plans.get_all_for_project(project.id) is called inside the for plan_name, plan_info in plans_data.items(): loop (line 106), causing N+1 database queries
Fix: Move the call outside the loop so it executes once before iteration begins
Scope: Also checking for any other similar N+1 patterns in LegacyDataMigrator

Subtask Wave Plan:

Wave 1 (parallel): Fix the N+1 query + check for other N+1 patterns
Wave 2: Update/add Behave tests covering the migration loop behaviour
Wave 3: Run nox quality gates

Difficulty assessment: Low → starting at sonnet tier.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

Starting implementation on branch `fix/legacy-migrator-inefficient-get-all-for-project`. **Issue Analysis:** - Bug: `ctx.plans.get_all_for_project(project.id)` is called inside the `for plan_name, plan_info in plans_data.items():` loop (line 106), causing N+1 database queries - Fix: Move the call outside the loop so it executes once before iteration begins - Scope: Also checking for any other similar N+1 patterns in `LegacyDataMigrator` **Subtask Wave Plan:** - Wave 1 (parallel): Fix the N+1 query + check for other N+1 patterns - Wave 2: Update/add Behave tests covering the migration loop behaviour - Wave 3: Run nox quality gates Difficulty assessment: Low → starting at sonnet tier. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo referenced this issue

2026-04-05 07:21:47 +00:00

[CA-AUTO] UAT Pool Supervisor — v3.2.0–v3.7.0 — Session Tracker #3094

freemo referenced this issue from a commit

2026-04-05 08:28:53 +00:00

fix(database): move get_all_for_project call outside loop in LegacyDataMigrator

freemo referenced this issue from a pull request that will close it,

2026-04-05 08:30:25 +00:00

fix(database): move get_all_for_project call outside loop in LegacyDataMigrator #3249

freemo commented

2026-04-05 08:36:06 +00:00

Author

Owner

All subtasks complete. Quality gates passed. PR #3249 created.

Implementation Summary:

Subtask 1 ✅ — Moved ctx.plans.get_all_for_project(project.id) outside the for plan_name, plan_info in plans_data.items(): loop in LegacyDataMigrator.migrate_project_data. The call now executes exactly once before the loop, eliminating the N+1 query pattern.

Subtask 2 ✅ — Verified no other similar N+1 query patterns exist in LegacyDataMigrator. The only other database call in the method (ctx.projects.get_by_name) is already outside any loop.

Subtask 3 ✅ — Added new Behave scenario get_all_for_project is called only once for multiple plans in features/legacy_migrator_coverage.feature. The scenario patches the repository method at the class level to count invocations and asserts exactly 1 call when migrating 4 plans. All 26 existing scenarios continue to pass.

Subtask 4 ✅ — All nox stages pass: lint (ruff, 0 errors), typecheck (pyright, 0 errors), unit_tests (26 scenarios, 205 steps passed).

PR #3249 created on branch fix/legacy-migrator-inefficient-get-all-for-project. PR review and merge handled by continuous review stream.

Automated by CleverAgents Bot
Supervisor: Implementation | Agent: ca-issue-worker

All subtasks complete. Quality gates passed. PR #3249 created. **Implementation Summary:** **Subtask 1 ✅** — Moved `ctx.plans.get_all_for_project(project.id)` outside the `for plan_name, plan_info in plans_data.items():` loop in `LegacyDataMigrator.migrate_project_data`. The call now executes exactly once before the loop, eliminating the N+1 query pattern. **Subtask 2 ✅** — Verified no other similar N+1 query patterns exist in `LegacyDataMigrator`. The only other database call in the method (`ctx.projects.get_by_name`) is already outside any loop. **Subtask 3 ✅** — Added new Behave scenario `get_all_for_project is called only once for multiple plans` in `features/legacy_migrator_coverage.feature`. The scenario patches the repository method at the class level to count invocations and asserts exactly 1 call when migrating 4 plans. All 26 existing scenarios continue to pass. **Subtask 4 ✅** — All nox stages pass: `lint` (ruff, 0 errors), `typecheck` (pyright, 0 errors), `unit_tests` (26 scenarios, 205 steps passed). PR #3249 created on branch `fix/legacy-migrator-inefficient-get-all-for-project`. PR review and merge handled by continuous review stream. --- **Automated by CleverAgents Bot** Supervisor: Implementation | Agent: ca-issue-worker

freemo added a new dependency

2026-04-05 17:28:51 +00:00

#3249 fix(database): move get_all_for_project call outside loop in LegacyDataMigrator

freemo referenced this issue

2026-04-05 17:29:22 +00:00

[Automated] CleverAgents Build Session - 2026-04-05 #3377

freemo closed this issue

2026-04-05 21:08:51 +00:00

freemo referenced this issue from a commit

2026-04-05 21:08:52 +00:00

fix(database): move get_all_for_project call outside loop in LegacyDataMigrator