ruff check apps/api_v2/ apps/gcd/migrations/0072_story_reprint_api_v2_browse_indexes.pyruff format --check apps/api_v2/ apps/gcd/migrations/0072_story_reprint_api_v2_browse_indexes.pypython manage.py checkpython -m pytest apps/api_v2/tests/ -v --tb=shortpython -m pytest apps/gcd/tests/test_story.py apps/gcd/tests/test_reprint.py -q --tb=shortgcd.0072_story_reprint_api_v2_browse_indexes to the local production-copy Docker DB for measurement.title__icontains text search.https://github.com/GrandComicsDatabase/gcd-django/pull/716
(32 files)
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()
@gemini-code-assist[bot] commented on this pull request.
This pull request introduces new API v2 endpoints, filters, serializers, and tests for stories, reprints, and story arcs, along with database indexes to optimize browsing and delta sync queries. The code review feedback highlights three performance and robustness improvements: resolving a potential N+1 query bottleneck in the story arc serializer by prefetching reprint relations, optimizing database queries in the story viewset by removing an unnecessary SQL JOIN on the character model, and adding a defensive guard check in the feature object serializer to prevent a potential AttributeError when the feature type is null.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
In apps/api_v2/serializers/story_arcs.py:
> +def _primary_stories(story_arc): + """Return ordered primary story memberships for ``story_arc``.""" + stories = getattr(story_arc, 'active_story_arc_story_list', None) + if stories is None: + stories, _reprinted_stories = story_arc.stories() + return [ + story + for story in stories + if not _is_same_language_reprint(story, story_arc.language_id) + ]
In _primary_stories, if active_story_arc_story_list is not prefetched (i.e., stories is None), the code falls back to calling story_arc.stories(). However, story_arc.stories() does not prefetch the from_all_reprints relation. This causes _is_same_language_reprint to execute a separate database query for every story in the list to check for same-language reprints, leading to a severe N+1 query performance bottleneck.
To resolve this, we should explicitly prefetch from_all_reprints with origin_issue__series when stories is fetched dynamically.
def _primary_stories(story_arc): """Return ordered primary story memberships for 'story_arc'.""" stories = getattr(story_arc, 'active_story_arc_story_list', None) if stories is None: from django.db.models import Prefetch from apps.gcd.models import Reprint stories, _reprinted_stories = story_arc.stories() stories = stories.prefetch_related( Prefetch( 'from_all_reprints', queryset=Reprint.objects.select_related('origin_issue__series'), to_attr='active_story_arc_reprint_list', ) ) return [ story for story in stories if not _is_same_language_reprint(story, story_arc.language_id) ]
In apps/api_v2/views/stories.py:
> +ACTIVE_STORY_CHARACTER_PREFETCH = Prefetch( + 'appearing_characters', + queryset=StoryCharacter.objects.filter(deleted=False) + .select_related( + 'character__character', + 'role', + ) + .order_by( + 'character__sort_name', + 'character__id', + ), + to_attr='active_character_list', +)
In ACTIVE_STORY_CHARACTER_PREFETCH, the select_related call includes 'character__character'. However, the StorySerializer.get_characters method only accesses appearance.character.character_id (which is the local foreign key on CharacterNameDetail) and appearance.character.name (which is a field on CharacterNameDetail). It does not access any fields on the parent Character model itself.
By changing 'character__character' to 'character', we can avoid an unnecessary SQL JOIN with the heavily populated Character table, improving query performance.
-ACTIVE_STORY_CHARACTER_PREFETCH = Prefetch( - 'appearing_characters', - queryset=StoryCharacter.objects.filter(deleted=False) - .select_related( - 'character__character', - 'role', - ) - .order_by( - 'character__sort_name', - 'character__id', - ), - to_attr='active_character_list', -) +ACTIVE_STORY_CHARACTER_PREFETCH = Prefetch( + 'appearing_characters', + queryset=StoryCharacter.objects.filter(deleted=False) + .select_related( + 'character', + 'role', + ) + .order_by( + 'character__sort_name', + 'character__id', + ), + to_attr='active_character_list', +)
In apps/api_v2/serializers/stories.py:
> + def get_feature_type(self, obj):
+ """Return the minimal nested feature type reference."""
+ return {
+ 'id': obj.feature_type_id,
+ 'name': obj.feature_type.name,
+ }
In FeatureObjectSerializer.get_feature_type, obj.feature_type is accessed directly to retrieve its name. If feature_type is nullable or missing due to data integrity issues, this will raise an AttributeError.
To adhere to defensive programming practices, we should add a guard check to ensure obj.feature_type is not None before accessing its properties.
- def get_feature_type(self, obj):
- """Return the minimal nested feature type reference."""
- return {
- 'id': obj.feature_type_id,
- 'name': obj.feature_type.name,
- }
+ def get_feature_type(self, obj):
+ """Return the minimal nested feature type reference."""
+ if obj.feature_type_id is None or obj.feature_type is None:
+ return None
+ return {
+ 'id': obj.feature_type_id,
+ 'name': obj.feature_type.name,
+ }
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()
@DeusExTaco commented on this pull request.
In apps/api_v2/serializers/story_arcs.py:
> +def _primary_stories(story_arc): + """Return ordered primary story memberships for ``story_arc``.""" + stories = getattr(story_arc, 'active_story_arc_story_list', None) + if stories is None: + stories, _reprinted_stories = story_arc.stories() + return [ + story + for story in stories + if not _is_same_language_reprint(story, story_arc.language_id) + ]
I added a fallback Prefetch for from_all_reprints with origin_issue__series when the serializer is used outside the view-prefetched path, and added a direct serializer query-count regression test.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()
@DeusExTaco commented on this pull request.
In apps/api_v2/views/stories.py:
> +ACTIVE_STORY_CHARACTER_PREFETCH = Prefetch( + 'appearing_characters', + queryset=StoryCharacter.objects.filter(deleted=False) + .select_related( + 'character__character', + 'role', + ) + .order_by( + 'character__sort_name', + 'character__id', + ), + to_attr='active_character_list', +)
The serializer only needs CharacterNameDetail and role, so I changed the prefetch and fallback query set from character__character to character and added a test locking the select-related shape.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()
@DeusExTaco commented on this pull request.
In apps/api_v2/serializers/stories.py:
> + def get_feature_type(self, obj):
+ """Return the minimal nested feature type reference."""
+ return {
+ 'id': obj.feature_type_id,
+ 'name': obj.feature_type.name,
+ }
Addressed defensively. Feature.feature_type is non-null in the model, but the serializer now returns feature_type: null for malformed objects with a missing or dangling feature type, with coverage for both cases.
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()
@DeusExTaco pushed 3 commits.
—
View it on GitHub or unsubscribe.
Triage notifications, keep track of coding agent tasks and review pull requests on the go with GitHub Mobile for iOS and Android. Download it today!
You are receiving this because you are subscribed to this thread.![]()