From adb4cbaffde9abbf2ae54dff6bcd6aa41cdaca53 Mon Sep 17 00:00:00 2001 From: StillKnotKnown Date: Tue, 10 Feb 2026 15:00:07 +0200 Subject: [PATCH] fix: address follow-up PR review findings (FU2-QUAL-001/002/003) FU2-QUAL-001 (MEDIUM): Unconditionally restore sys.modules in finally block - Changed conditional restoration to unconditional to ensure broken modules from failed exec_module() calls don't persist in sys.modules FU2-QUAL-002 (MEDIUM): Remove test dependencies from production requirements - Removed pytest>=8.0.0 and pytest-cov>=5.0.0 from apps/backend/requirements.txt - Test dependencies already exist in tests/requirements-test.txt FU2-QUAL-003 (LOW): Add pagination to GitLab notes API call - Added pagination loop to fetch all issue notes before filtering - Prevents selected notes from being silently dropped when they're beyond the default 20-item page limit --- apps/backend/requirements.txt | 4 --- .../gitlab/investigation-handlers.ts | 31 +++++++++++++++---- tests/test_cli_build_commands.py | 5 ++- 3 files changed, 27 insertions(+), 13 deletions(-) diff --git a/apps/backend/requirements.txt b/apps/backend/requirements.txt index cfc2dac1..2cd44d8e 100644 --- a/apps/backend/requirements.txt +++ b/apps/backend/requirements.txt @@ -31,9 +31,5 @@ google-generativeai>=0.8.0 # Pydantic for structured output schemas pydantic>=2.0.0 -# Testing -pytest>=8.0.0 -pytest-cov>=5.0.0 - # Error tracking (optional - requires SENTRY_DSN environment variable) sentry-sdk>=2.0.0 diff --git a/apps/frontend/src/main/ipc-handlers/gitlab/investigation-handlers.ts b/apps/frontend/src/main/ipc-handlers/gitlab/investigation-handlers.ts index 727efa62..1db9da24 100644 --- a/apps/frontend/src/main/ipc-handlers/gitlab/investigation-handlers.ts +++ b/apps/frontend/src/main/ipc-handlers/gitlab/investigation-handlers.ts @@ -109,14 +109,33 @@ export function registerInvestigateIssue( `/projects/${encodedProject}/issues/${issueIid}` ) as GitLabAPIIssue; - // Fetch notes if any selected + // Fetch notes if any selected (with pagination to get all notes) let filteredNotes: Array<{ body: string; author: { username: string } }> = []; if (selectedNoteIds && selectedNoteIds.length > 0) { - const allNotes = await gitlabFetch( - config.token, - config.instanceUrl, - `/projects/${encodedProject}/issues/${issueIid}/notes` - ) as Array<{ id: number; body: string; author: { username: string } }>; + // Fetch all notes with pagination (GitLab defaults to 20 per page) + const allNotes: Array<{ id: number; body: string; author: { username: string } }> = []; + let page = 1; + const perPage = 100; + let hasMore = true; + + while (hasMore) { + const notesPage = await gitlabFetch( + config.token, + config.instanceUrl, + `/projects/${encodedProject}/issues/${issueIid}/notes?page=${page}&per_page=${perPage}` + ) as Array<{ id: number; body: string; author: { username: string } }>; + + if (notesPage.length === 0) { + hasMore = false; + } else { + allNotes.push(...notesPage); + if (notesPage.length < perPage) { + hasMore = false; + } else { + page++; + } + } + } // Filter notes based on selection filteredNotes = allNotes.filter(note => selectedNoteIds.includes(note.id)); diff --git a/tests/test_cli_build_commands.py b/tests/test_cli_build_commands.py index cdcc50b5..302ecd02 100644 --- a/tests/test_cli_build_commands.py +++ b/tests/test_cli_build_commands.py @@ -2594,7 +2594,6 @@ class TestBuildCommandsModuleImport: finally: # Always restore original state, even if an exception occurred sys.path[:] = original_path - # Restore saved modules (if they still exist in sys.modules, skip to avoid conflicts) + # Unconditionally restore saved modules (overwrite to ensure clean state after failures) for mod_name, mod_obj in original_modules.items(): - if mod_name not in sys.modules: - sys.modules[mod_name] = mod_obj + sys.modules[mod_name] = mod_obj