Skip to content

Avoid N+1 queries when rendering llms-full.txt - #491

Merged
thibaudcolas merged 1 commit into
wagtail:mainfrom
ShubhJain09:fix-481-llms-n-plus-one
Oct 5, 2026
Merged

thibaudcolas merged 1 commit into
wagtail:mainfrom
ShubhJain09:fix-481-llms-n-plus-one

Conversation

@ShubhJain09

@ShubhJain09 ShubhJain09 commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Refs #481

Description

/llms-full.txt was running an extra query for every ContentPage just to load its body
The pages come from Sitemap.items() which calls defer_streamfields()
That's fine for llms.txt but llms-full.txt renders every page body so each one was fetched on its own

Both views now get their pages from one shared helper and only llms.txt defers StreamFields
So llms-full.txt loads all the bodies in one go

With the fixture data of 24 content pages the request went from 75 queries to 50 and body queries dropped from 24 to 1
The output is exactly the same

I added a regression test that creates four ContentPages and checks that the bodies come from a single query and all show up in the response
It fails on main with 4 body queries

I ran the llms_txt tests plus ruff check and ruff format --check locally

AI usage

None

Copilot AI lite review requested due to automatic review settings September 25, 2026 06:14

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🔵 Needs a closer look

The regression test should verify that page body content is rendered, not only the query count.

Review effort: Lite
Findings: None

What changed in this PR

Updates /llms-full.txt to avoid per-page body queries while preserving existing llms.txt behavior.

Changes:

  • Loads full page content eagerly.
  • Adds regression coverage for query counts and multiple pages.
File Summary
apps/​llms_txt/​views.py Uses eager StreamField loading for full output.
apps/​llms_txt/​tests/​test_views.py Tests query behavior across multiple pages; should also assert rendered body content.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@ShubhJain09
ShubhJain09 force-pushed the fix-481-llms-n-plus-one branch from 3a15801 to f2d1166 Compare October 4, 2026 12:02
@ShubhJain09

Copy link
Copy Markdown
Contributor Author

Thanks this is addressed now
The test checks that every page body is rendered as well as the query count

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

The implementation preserves existing page selection while resolving the N+1 query pattern with focused regression coverage.

Review effort: Balanced
Findings: None

@ShubhJain09

Copy link
Copy Markdown
Contributor Author

Hi @thibaudcolas, this is ready for review whenever you have time. It handles the "reduce overfetching" part of #481 for /llms-full.txt (75 → 50 queries on the fixture data, with body queries going from 24 to 1).
The CI workflow is waiting for approval to run, since this is my first contribution here. Happy to make any changes.

Reconcile defer_streamfields page loading with skill_name template context
added on main, and keep the query-count regression test.

Assisted-by: Cursor <cursoragent@cursor.com>
@thibaudcolas
thibaudcolas force-pushed the fix-481-llms-n-plus-one branch from f2d1166 to 153fa86 Compare October 5, 2026 10:52
@thibaudcolas
thibaudcolas merged commit acf641d into wagtail:main Oct 5, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants