[GrandComicsDatabase/gcd-django] API v2 phase 3 stories, reprints, and story arcs (PR #716)

3 views
Skip to first unread message

Adam Hernandez

unread,
Jun 9, 2026, 5:59:12 PM (2 days ago) Jun 9
to GrandComicsDatabase/gcd-django, Subscribed

Summary

  • Add public API v2 endpoints for stories, reprints, and story arcs, plus structured story credit output.
  • Keep the Sprint 3 routes on the public surface only and absent from MYCOMICS routing/schema.
  • Add browse indexes and modified-delta no-count pagination for the heaviest story/reprint list paths.

Validation

  • ruff check apps/api_v2/ apps/gcd/migrations/0072_story_reprint_api_v2_browse_indexes.py
  • ruff format --check apps/api_v2/ apps/gcd/migrations/0072_story_reprint_api_v2_browse_indexes.py
  • python manage.py check
  • python -m pytest apps/api_v2/tests/ -v --tb=short
  • python -m pytest apps/gcd/tests/test_story.py apps/gcd/tests/test_reprint.py -q --tb=short

Manual performance notes

  • Applied migration gcd.0072_story_reprint_api_v2_browse_indexes to the local production-copy Docker DB for measurement.
  • Optimized JSON harness runs: stories list about 306 ms, stories page 5 about 284 ms, stories modified delta about 11 ms.
  • Optimized JSON harness runs: reprints list about 126 ms, reprints page 5 about 90 ms, reprints modified delta about 14 ms.
  • Remaining known slow path: exact count for title__icontains text search.

You can view, comment on, or merge this pull request online at:

  https://github.com/GrandComicsDatabase/gcd-django/pull/716

Commit Summary

  • 6270867 feat(api-v2): add structured story credit helper
  • d76579b feat(api-v2): add stories endpoint
  • 0b0944c feat(api-v2): add reprints endpoint
  • ec8ddb5 feat(api-v2): add story arcs endpoint
  • 2c2046e feat(api-v2): optimize story and reprint browse

File Changes

(32 files)

Patch Links:


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.Message ID: <GrandComicsDatabase/gcd-django/pull/716@github.com>

gemini-code-assist[bot]

unread,
Jun 9, 2026, 6:02:06 PM (2 days ago) Jun 9
to GrandComicsDatabase/gcd-django, Subscribed

@gemini-code-assist[bot] commented on this pull request.

Code Review

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)
+    ]

high

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',
+)

medium

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.

⬇️ Suggested change
-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,
+        }

medium

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.

⬇️ Suggested change
-    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.Message ID: <GrandComicsDatabase/gcd-django/pull/716/review/4462761248@github.com>

Adam Hernandez

unread,
Jun 9, 2026, 6:25:53 PM (2 days ago) Jun 9
to GrandComicsDatabase/gcd-django, Subscribed

@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.Message ID: <GrandComicsDatabase/gcd-django/pull/716/review/4462903289@github.com>

Adam Hernandez

unread,
Jun 9, 2026, 6:26:33 PM (2 days ago) Jun 9
to GrandComicsDatabase/gcd-django, Subscribed

@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.Message ID: <GrandComicsDatabase/gcd-django/pull/716/review/4462906220@github.com>

Adam Hernandez

unread,
Jun 9, 2026, 6:27:07 PM (2 days ago) Jun 9
to GrandComicsDatabase/gcd-django, Subscribed

@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.Message ID: <GrandComicsDatabase/gcd-django/pull/716/review/4462908703@github.com>

Adam Hernandez

unread,
Jun 9, 2026, 6:27:54 PM (2 days ago) Jun 9
to GrandComicsDatabase/gcd-django, Push

@DeusExTaco pushed 3 commits.

  • f8ae628 fix(api-v2): prefetch story arc fallback reprints
  • a0d07cb fix(api-v2): trim story character prefetch join
  • e4d7b1b fix(api-v2): guard missing story feature types


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.Message ID: <GrandComicsDatabase/gcd-django/pull/716/before/2c2046e0dee8b5f4b54eae488af6c7ab38d62c39/after/e4d7b1b408a07ae5ac2d5524043f228c22fcedbb@github.com>

Reply all
Reply to author
Forward
0 new messages